Skip to content

Commit b742c94

Browse files
gkzmeta-codesync[bot]
authored andcommitted
[flow][records] Disallow record names starting with lowercase 'a'-'z'
Summary: Disallow record names starting with lowercase 'a'-'z'. We would have this as part of the style guide anyway, but update here: - Record expressions (e.g. `foo {}`): do not parse at all - We want to future proof the syntax for new contextual keywords being added in TC39 proposals - Prevent parsing of record expressions in the middle of writing/editing existing constructs like `clas {}` - Record declarations (e.g. `record foo {}`: add a Flow error - No syntax conflicts, so we parse and Flow error with an explanation Changelog: [internal] Reviewed By: SamChou19815 Differential Revision: D89772551 fbshipit-source-id: 9d5689cb4b77ef63c70a0300f9b4a197994e216c
1 parent 8e59126 commit b742c94

File tree

12 files changed

+93
-3
lines changed

12 files changed

+93
-3
lines changed

src/common/errors/error_codes.ml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ type error_code =
136136
| ReassignEnum
137137
| ReassignImport
138138
| RecordBannedTypeUtil
139+
| RecordInvalidName
139140
| RecordInvalidNew
140141
| RecordInvalidSyntax
141142
| ReferenceBeforeDeclaration
@@ -355,6 +356,7 @@ let string_of_code : error_code -> string = function
355356
| ReassignEnum -> "reassign-enum"
356357
| ReassignImport -> "reassign-import"
357358
| RecordBannedTypeUtil -> "record-banned-type-util"
359+
| RecordInvalidName -> "record-invalid-name"
358360
| RecordInvalidNew -> "record-invalid-new"
359361
| RecordInvalidSyntax -> "record-invalid-syntax"
360362
| ReferenceBeforeDeclaration -> "reference-before-declaration"

src/parser/expression_parser.ml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -837,15 +837,17 @@ module Expression
837837
let in_optional_chain = Option.is_some optional in
838838
call_cover ~allow_optional_chain ~in_optional_chain env start_loc (Cover_expr (loc, call))
839839
in
840+
let is_a_to_z c = c >= 'a' && c <= 'z' in
840841
let should_parse_record env constructor =
841842
(parse_options env).records
842843
&& (not (Peek.ith_is_line_terminator ~i:1 env))
843844
&& (not (no_record env))
844845
&&
845846
match constructor with
846-
| (_, Expression.Identifier _)
847-
| (_, Expression.Member _) ->
847+
| (_, Expression.Identifier (_, { Identifier.name; _ }))
848+
when name != "" && not (is_a_to_z name.[0]) ->
848849
true
850+
| (_, Expression.Member _) -> true
849851
| _ -> false
850852
in
851853
let parse_record env ~constructor ~targs =
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// Not a `RecordExpression`
2+
clas {}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"records": true
3+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
{
2+
"errors":[
3+
{
4+
"loc":{"source":null,"start":{"line":2,"column":5},"end":{"line":2,"column":6}},
5+
"message":"Unexpected token `{`, expected the end of an expression statement (`;`)"
6+
}
7+
],
8+
"type":"Program",
9+
"loc":{"source":null,"start":{"line":2,"column":0},"end":{"line":2,"column":7}},
10+
"range":[28,35],
11+
"body":[
12+
{
13+
"type":"ExpressionStatement",
14+
"loc":{"source":null,"start":{"line":2,"column":0},"end":{"line":2,"column":4}},
15+
"range":[28,32],
16+
"expression":{
17+
"type":"Identifier",
18+
"loc":{"source":null,"start":{"line":2,"column":0},"end":{"line":2,"column":4}},
19+
"range":[28,32],
20+
"name":"clas",
21+
"typeAnnotation":null,
22+
"optional":false
23+
},
24+
"directive":null
25+
},
26+
{
27+
"type":"BlockStatement",
28+
"loc":{"source":null,"start":{"line":2,"column":5},"end":{"line":2,"column":7}},
29+
"range":[33,35],
30+
"body":[]
31+
}
32+
],
33+
"comments":[
34+
{
35+
"type":"Line",
36+
"loc":{"source":null,"start":{"line":1,"column":0},"end":{"line":1,"column":27}},
37+
"range":[0,27],
38+
"value":" Not a `RecordExpression`"
39+
}
40+
]
41+
}

src/typing/debug_js.ml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2059,6 +2059,8 @@ let dump_error_message =
20592059
spf "EMatchInvalidInstancePattern (%s)" (string_of_aloc loc)
20602060
| ERecordBannedTypeUtil { reason_op; reason_record } ->
20612061
spf "ERecordBannedTypeUtil (%s) (%s)" (dump_reason cx reason_op) (dump_reason cx reason_record)
2062+
| ERecordInvalidName { name; loc } ->
2063+
spf "ERecordInvalidName { name = %s; loc = %s }" name (string_of_aloc loc)
20622064
| ERecordInvalidNew { record_name; loc } ->
20632065
spf "ERecordInvalidNew { record_name = %s; loc = %s }" record_name (string_of_aloc loc)
20642066
| ERecordDeclarationInvalidSyntax { loc; _ } ->

src/typing/errors/error_message.ml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -767,6 +767,10 @@ and 'loc t' =
767767
reason_op: 'loc virtual_reason;
768768
reason_record: 'loc virtual_reason;
769769
}
770+
| ERecordInvalidName of {
771+
loc: 'loc;
772+
name: string;
773+
}
770774
| ERecordInvalidNew of {
771775
loc: 'loc;
772776
record_name: string;
@@ -1889,6 +1893,7 @@ let rec map_loc_of_error_message (f : 'a -> 'b) : 'a t' -> 'b t' =
18891893
ERecordBannedTypeUtil
18901894
{ reason_op = map_reason reason_op; reason_record = map_reason reason_record }
18911895
| ERecordInvalidNew { loc; record_name } -> ERecordInvalidNew { loc = f loc; record_name }
1896+
| ERecordInvalidName { loc; name } -> ERecordInvalidName { loc = f loc; name }
18921897
| ERecordDeclarationInvalidSyntax { loc; kind } ->
18931898
let kind =
18941899
match kind with
@@ -2302,6 +2307,7 @@ let util_use_op_of_msg nope util = function
23022307
| EMatchInvalidInstancePattern _
23032308
| ERecordBannedTypeUtil _
23042309
| ERecordInvalidNew _
2310+
| ERecordInvalidName _
23052311
| ERecordDeclarationInvalidSyntax _
23062312
| EUndocumentedFeature _
23072313
| EIllegalAssertOperator _ ->
@@ -2544,6 +2550,7 @@ let loc_of_msg : 'loc t' -> 'loc option = function
25442550
| EMatchInvalidGuardedWildcard loc -> Some loc
25452551
| EMatchInvalidIdentOrMemberPattern { loc; _ } -> Some loc
25462552
| ERecordInvalidNew { loc; _ } -> Some loc
2553+
| ERecordInvalidName { loc; _ } -> Some loc
25472554
| ERecordDeclarationInvalidSyntax { loc; _ } -> Some loc
25482555
| EUndocumentedFeature { loc } -> Some loc
25492556
| EDevOnlyRefinedLocInfo { refined_loc; refining_locs = _ } -> Some refined_loc
@@ -3766,6 +3773,7 @@ let friendly_message_of_msg = function
37663773
| ERecordBannedTypeUtil { reason_op; reason_record } ->
37673774
Normal (MessageRecordBannedTypeUtil { reason_op; reason_record })
37683775
| ERecordInvalidNew { record_name; loc = _ } -> Normal (MessageRecordInvalidNew { record_name })
3776+
| ERecordInvalidName { name; loc = _ } -> Normal (MessageRecordInvalidName { name })
37693777
| ERecordDeclarationInvalidSyntax { loc = _; kind } ->
37703778
Normal (MessageRecordDeclarationInvalidSyntax kind)
37713779
| EUndocumentedFeature { loc = _ } -> Normal MessageUndocumentedFeature
@@ -4178,6 +4186,7 @@ let error_code_of_message err : error_code option =
41784186
| EMatchInvalidInstancePattern _ -> Some MatchInvalidPattern
41794187
| ERecordBannedTypeUtil _ -> Some RecordBannedTypeUtil
41804188
| ERecordInvalidNew _ -> Some RecordInvalidNew
4189+
| ERecordInvalidName _ -> Some RecordInvalidName
41814190
| ERecordDeclarationInvalidSyntax _ -> Some RecordInvalidSyntax
41824191
| EUndocumentedFeature _ -> Some UndocumentedFeature
41834192
| EIllegalAssertOperator _ -> Some IllegalAssertOperator

src/typing/errors/flow_intermediate_error.ml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5465,6 +5465,12 @@ let to_printable_error :
54655465
code (spf "%s {...}" record_name);
54665466
text ".";
54675467
]
5468+
| MessageRecordInvalidName { name } ->
5469+
[
5470+
text "Invalid record name ";
5471+
code name;
5472+
text ". Record names cannot start with lowercase 'a' through 'z'.";
5473+
]
54685474
| MessageRecordDeclarationInvalidSyntax kind ->
54695475
let msg_invalid_infix_equals =
54705476
[text "Record declarations don't need the "; code "="; text ", remove it."]

src/typing/errors/flow_intermediate_error_types.ml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1198,6 +1198,7 @@ type 'loc message =
11981198
reason_record: 'loc virtual_reason;
11991199
}
12001200
| MessageRecordInvalidNew of { record_name: string }
1201+
| MessageRecordInvalidName of { name: string }
12011202
| MessageRecordDeclarationInvalidSyntax of 'loc record_declaration_invalid_syntax
12021203
| MessageConstantCondition of {
12031204
is_truthy: bool;

src/typing/statement.ml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1816,6 +1816,9 @@ module Make
18161816
~multiple_error_loc:name_loc
18171817
invalid_syntax
18181818
invalid_properties_syntax;
1819+
let is_a_to_z c = c >= 'a' && c <= 'z' in
1820+
if name != "" && is_a_to_z name.[0] then
1821+
Flow_js_utils.add_output cx (Error_message.ERecordInvalidName { loc = name_loc; name });
18191822
let name = OrdinaryName name in
18201823
let reason = DescFormat.instance_reason name name_loc in
18211824
let tast_record_type = Type_env.read_declared_type cx reason name_loc in

0 commit comments

Comments
 (0)