Skip to content

Commit 5b391d6

Browse files
vassilmladenovfacebook-github-bot
authored andcommitted
Eliminate SFregex_group shape AST node
Summary: This node represents int keys in shape literals, which is a parse error. We retain the associated type `TSFregex_group` which is created by capture groups in `re`-prefixed strings. Differential Revision: D67779741 fbshipit-source-id: 0be9a7292be1b6a5982f6222468eafbc3c2537ad
1 parent bc09721 commit 5b391d6

File tree

26 files changed

+16
-157
lines changed

26 files changed

+16
-157
lines changed

hphp/hack/src/ast/ast_defs.ml

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,6 @@ and byte_string = string
2929
and positioned_byte_string = pos * byte_string
3030

3131
and shape_field_name =
32-
| SFregex_group of pstring
33-
(** TODO(T199271494) Eliminate this group node and its supporting code. It
34-
* is conjured by typing for inference of Shapes::idx for regex results,
35-
* but is otherwise never emitted. *)
3632
| SFlit_str of positioned_byte_string
3733
| SFclassname of id (* nameof C *)
3834
| SFclass_const of id * pstring (* C::X *)
@@ -349,7 +345,6 @@ module ShapeField = struct
349345
* we have to write our own compare. *)
350346
let compare x y =
351347
match (x, y) with
352-
| (SFregex_group (_, s1), SFregex_group (_, s2)) -> String.compare s1 s2
353348
| (SFlit_str (_, s1), SFlit_str (_, s2)) -> String.compare s1 s2
354349
| (SFclassname (_, s1), SFclassname (_, s2)) -> String.compare s1 s2
355350
| (SFclass_const ((_, s1), (_, s1')), SFclass_const ((_, s2), (_, s2'))) ->
@@ -358,27 +353,18 @@ module ShapeField = struct
358353
~cmp2:String.compare
359354
(s1, s1')
360355
(s2, s2')
361-
(* Cross-variant order: SFregex_group, SFlit_str, SFclassname, SFclass_const
356+
(* Cross-variant order: SFlit_str, SFclassname, SFclass_const
362357
*
363358
* This can be derived provided that we validate a deriver in Rust that is
364359
* synced to this. Rust also does not have facilities like [@compare.ignore]
365360
* so we need a special Pos that always compares to 0. *)
366-
| (SFregex_group _, SFlit_str _)
367-
| (SFregex_group _, SFclassname _)
368-
| (SFregex_group _, SFclass_const _) ->
369-
-1
370-
(* spacing *)
371-
| (SFlit_str _, SFregex_group _) -> 1
372361
| (SFlit_str _, SFclassname _)
373362
| (SFlit_str _, SFclass_const _) ->
374363
-1
375364
(* spacing *)
376-
| (SFclassname _, SFregex_group _)
377-
| (SFclassname _, SFlit_str _) ->
378-
1
365+
| (SFclassname _, SFlit_str _) -> 1
379366
| (SFclassname _, SFclass_const _) -> -1
380367
(* spacing *)
381-
| (SFclass_const _, SFregex_group _)
382368
| (SFclass_const _, SFlit_str _)
383369
| (SFclass_const _, SFclassname _) ->
384370
1

hphp/hack/src/client/ide_service/code_actions_services/refactors/flip_around_comma.ml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ let pos_of_expr = Tuple3.get2
7979
let pos_of_shape_field_name =
8080
Ast_defs.(
8181
function
82-
| SFregex_group (pos, _) -> pos
8382
| SFlit_str (pos, _) -> pos
8483
| SFclassname (pos, _) -> pos
8584
| SFclass_const (_, (pos, _)) -> pos)

hphp/hack/src/decl/direct_decl_smart_constructors.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2254,9 +2254,6 @@ impl<'a, 'o, 't, S: SourceTextAllocator<'t, 'a>> DirectDeclSmartConstructors<'a,
22542254

22552255
fn make_t_shape_field_name(&self, ShapeField(field): &ShapeField<'a>) -> TShapeField<'a> {
22562256
TShapeField(match field {
2257-
ShapeFieldName::SFregexGroup(&(pos, x)) => {
2258-
TshapeFieldName::TSFregexGroup(self.alloc(PosString(pos, x)))
2259-
}
22602257
ShapeFieldName::SFlitStr(&(pos, x)) => {
22612258
TshapeFieldName::TSFlitStr(self.alloc(PosByteString(pos, x)))
22622259
}

hphp/hack/src/elab/passes/validate_shape_name.rs

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -30,41 +30,3 @@ fn error_if_duplicate_names(flds: &[(ShapeFieldName, Expr)], env: &Env) {
3030
seen.insert(name);
3131
}
3232
}
33-
34-
#[cfg(test)]
35-
mod tests {
36-
use nast::Expr;
37-
use nast::Pos;
38-
use oxidized::typechecker_options::TypecheckerOptions;
39-
40-
use super::*;
41-
use crate::env::ProgramSpecificOptions;
42-
43-
fn mk_shape_lit_int_field_name(name: &str) -> ShapeFieldName {
44-
ShapeFieldName::SFregexGroup((Pos::NONE, name.to_string()))
45-
}
46-
47-
fn mk_shape(flds: Vec<(ShapeFieldName, Expr)>) -> Expr {
48-
Expr((), Pos::NONE, Expr_::Shape(flds))
49-
}
50-
51-
#[test]
52-
fn test_duplicate_field_names() {
53-
let env = Env::new(
54-
&TypecheckerOptions::default(),
55-
&ProgramSpecificOptions::default(),
56-
);
57-
let f = mk_shape_lit_int_field_name("foo");
58-
let mut shape = mk_shape(vec![
59-
(f.clone(), elab_utils::expr::null()),
60-
(f, elab_utils::expr::null()),
61-
]);
62-
shape.transform(&env, &mut ValidateShapeNamePass);
63-
assert!(matches!(
64-
&env.into_errors()[..],
65-
[NamingPhaseError::Naming(
66-
NamingError::FieldNameAlreadyBound { .. }
67-
)]
68-
));
69-
}
70-
}

hphp/hack/src/hackc/emitter/constant_folder.rs

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -220,18 +220,6 @@ fn shape_to_typed_value(
220220
.iter()
221221
.map(|(sf, expr)| {
222222
let key = match sf {
223-
ast_defs::ShapeFieldName::SFregexGroup((_, s)) => {
224-
let tv = int_expr_to_typed_value(s)?;
225-
match tv {
226-
TypedValue::Int(_) => tv,
227-
_ => {
228-
return Err(Error::unrecoverable(format!(
229-
"{} is not a valid integer index",
230-
s
231-
)));
232-
}
233-
}
234-
}
235223
ast_defs::ShapeFieldName::SFlitStr(id) => TypedValue::intern_string(&id.1),
236224
ast_defs::ShapeFieldName::SFclassname(class_id) => nameof_to_typed_value(
237225
emitter,

hphp/hack/src/hackc/emitter/emit_expression.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1106,7 +1106,6 @@ fn extract_shape_field_name_pstring<'a>(
11061106
) -> Result<ast::Expr_> {
11071107
use ast_defs::ShapeFieldName as SF;
11081108
Ok(match field {
1109-
SF::SFregexGroup(s) => ast::Expr_::mk_int(s.1.clone()),
11101109
SF::SFlitStr(s) => ast::Expr_::mk_string(s.1.clone()),
11111110
SF::SFclassname(id) => {
11121111
ast::Expr_::mk_nameof(ast::ClassId((), pos.clone(), ast::ClassId_::CI(id.clone())))

hphp/hack/src/hackc/emitter/emit_property.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -251,9 +251,7 @@ fn expr_requires_deep_init(ast::Expr(_, _, expr): &ast::Expr, force_class_init:
251251

252252
fn shape_field_requires_deep_init((name, expr): &(ast_defs::ShapeFieldName, ast::Expr)) -> bool {
253253
match name {
254-
ast_defs::ShapeFieldName::SFregexGroup(_) | ast_defs::ShapeFieldName::SFlitStr(_) => {
255-
expr_requires_deep_init_(expr)
256-
}
254+
ast_defs::ShapeFieldName::SFlitStr(_) => expr_requires_deep_init_(expr),
257255
ast_defs::ShapeFieldName::SFclassname(_) => false, // dynamic names are banned
258256
ast_defs::ShapeFieldName::SFclassConst(ast_defs::Id(_, s), (_, p)) => {
259257
class_const_requires_deep_init(s, p)

hphp/hack/src/hackc/emitter/emit_type_constant.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@ fn is_prim_or_resolved_classname(kind: TypeStructureKind) -> bool {
105105
fn shape_field_name(sf: &ShapeFieldName) -> (String, bool) {
106106
use oxidized::ast_defs::Id;
107107
match sf {
108-
ShapeFieldName::SFregexGroup((_, s)) => (s.to_string(), false),
109108
ShapeFieldName::SFlitStr((_, s)) => {
110109
(
111110
// FIXME: This is not safe--string literals are binary strings.

hphp/hack/src/hackc/emitter/emit_type_hint.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,6 @@ pub fn fmt_hint(tparams: &[&str], strip_tparams: bool, hint: &Hint) -> Result<St
128128
// TODO: Check whether shape fields need to retain order *)
129129
Hshape(NastShapeInfo { field_map, .. }) => {
130130
let fmt_field_name = |name: &ShapeFieldName| match name {
131-
ShapeFieldName::SFregexGroup((_, s)) => s.to_owned(),
132131
ShapeFieldName::SFlitStr((_, s)) => format!("'{}'", s),
133132
ShapeFieldName::SFclassname(Id(_, cid)) => {
134133
format!("'{}'", fmt_name_or_prim(tparams, cid))

hphp/hack/src/hackc/print_expr/print.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,6 @@ fn print_expr(
293293
) -> Result<()> {
294294
use ast::ShapeFieldName as S;
295295
match field {
296-
S::SFregexGroup((_, s)) => print_expr_int(w, s.as_ref()),
297296
S::SFlitStr((_, s)) => print_expr_string(w, s),
298297
S::SFclassname(id) => write::wrap_by(w, "\"", |w| print_expr_id(w, env, id.1.as_ref())),
299298
// TODO(T204024412) This is broken, it loses info about the class

0 commit comments

Comments
 (0)