From 9ebe9e49ef3be634ac306fceaae59774fbe72363 Mon Sep 17 00:00:00 2001 From: James Wu Date: Thu, 14 Jul 2022 12:42:19 -0700 Subject: [PATCH] Module Newtype: Correctly declare and lower module newtypes to the new visibility Summary: This diff adds declaration and lowering to enable module-level newtypes, completing the feature in the typechecker. After this diff, module level newtypes will behave properly in the typechecker. At runtime, they act like regular type aliases and behave properly as such. Reviewed By: hgoldstein Differential Revision: D37467891 fbshipit-source-id: 616c47f58f62cd11b30a7aab7cd2cd917b7b3559 --- .../decl/direct_decl_smart_constructors.rs | 6 +++- hphp/hack/src/hackc/emitter/emit_typedef.rs | 2 +- hphp/hack/src/parser/lowerer/lowerer.rs | 2 ++ .../fanout/modules/module-remove-decl.php | 4 +-- ...dule-remove-decl.php.incr_no_old_decls.exp | 1 + .../module-remove-decl.php.incr_old_decl.exp | 1 + .../module-remove-decl.php.saved_state.exp | 1 + .../test/typecheck/modules/module_newtype.php | 33 +++++++++++++++++++ .../typecheck/modules/module_newtype.php.exp | 8 +++++ hphp/test/slow/modules/module_newtype.php | 25 ++++++++++++++ .../slow/modules/module_newtype.php.expectf | 3 ++ .../slow/modules/module_newtype_module.inc | 3 ++ .../modules/module_newtype_outside_module.inc | 5 +++ .../modules/module_newtype_separate_file.inc | 6 ++++ 14 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 hphp/hack/test/typecheck/modules/module_newtype.php create mode 100644 hphp/hack/test/typecheck/modules/module_newtype.php.exp create mode 100644 hphp/test/slow/modules/module_newtype.php create mode 100644 hphp/test/slow/modules/module_newtype.php.expectf create mode 100644 hphp/test/slow/modules/module_newtype_module.inc create mode 100644 hphp/test/slow/modules/module_newtype_outside_module.inc create mode 100644 hphp/test/slow/modules/module_newtype_separate_file.inc diff --git a/hphp/hack/src/decl/direct_decl_smart_constructors.rs b/hphp/hack/src/decl/direct_decl_smart_constructors.rs index 9f9fb4b7f8a2b..687166841aca3 100644 --- a/hphp/hack/src/decl/direct_decl_smart_constructors.rs +++ b/hphp/hack/src/decl/direct_decl_smart_constructors.rs @@ -3070,7 +3070,7 @@ impl<'a, 'o, 't, S: SourceTextAllocator<'t, 'a>> FlattenSmartConstructors &mut self, attributes: Self::Output, modifiers: Self::Output, - _module_kw_opt: Self::Output, + module_kw_opt: Self::Output, keyword: Self::Output, name: Self::Output, generic_params: Self::Output, @@ -3111,11 +3111,15 @@ impl<'a, 'o, 't, S: SourceTextAllocator<'t, 'a>> FlattenSmartConstructors let internal = modifiers .iter() .any(|m| m.as_visibility() == Some(aast::Visibility::Internal)); + let is_module_newtype = module_kw_opt.is_ignored_token_with_kind(TokenKind::Module); let typedef = self.alloc(TypedefType { module: self.module, pos, vis: match keyword.token_kind() { Some(TokenKind::Type) => aast::TypedefVisibility::Transparent, + Some(TokenKind::Newtype) if is_module_newtype => { + aast::TypedefVisibility::OpaqueModule + } Some(TokenKind::Newtype) => aast::TypedefVisibility::Opaque, _ => aast::TypedefVisibility::Transparent, }, diff --git a/hphp/hack/src/hackc/emitter/emit_typedef.rs b/hphp/hack/src/hackc/emitter/emit_typedef.rs index cb4fbb4288e82..53bcda5a3513e 100644 --- a/hphp/hack/src/hackc/emitter/emit_typedef.rs +++ b/hphp/hack/src/hackc/emitter/emit_typedef.rs @@ -46,7 +46,7 @@ fn emit_typedef<'a, 'arena, 'decl>( emitter, &tparams, typedef.kind.clone(), - typedef.vis.is_opaque(), + typedef.vis.is_opaque() || typedef.vis.is_opaque_module(), ); let span = HhasSpan::from_pos(&typedef.span); let mut attrs = Attr::AttrNone; diff --git a/hphp/hack/src/parser/lowerer/lowerer.rs b/hphp/hack/src/parser/lowerer/lowerer.rs index 595f1467ca5c6..9ceb690ae7c92 100644 --- a/hphp/hack/src/parser/lowerer/lowerer.rs +++ b/hphp/hack/src/parser/lowerer/lowerer.rs @@ -5294,6 +5294,7 @@ fn p_def<'a>(node: S<'a>, env: &mut Env<'a>) -> Result> { AliasDeclaration(c) => { let tparams = p_tparam_l(false, &c.generic_parameter, env)?; let kinds = p_kinds(&c.modifiers, env)?; + let is_module_newtype = !c.module_kw_opt.is_missing(); for tparam in tparams.iter() { if tparam.reified != ast::ReifyKind::Erased { raise_parsing_error(node, env, &syntax_error::invalid_reified) @@ -5315,6 +5316,7 @@ fn p_def<'a>(node: S<'a>, env: &mut Env<'a>) -> Result> { mode: env.file_mode(), vis: match token_kind(&c.keyword) { Some(TK::Type) => ast::TypedefVisibility::Transparent, + Some(TK::Newtype) if is_module_newtype => ast::TypedefVisibility::OpaqueModule, Some(TK::Newtype) => ast::TypedefVisibility::Opaque, _ => missing_syntax("kind", &c.keyword, env)?, }, diff --git a/hphp/hack/test/fanout/modules/module-remove-decl.php b/hphp/hack/test/fanout/modules/module-remove-decl.php index 42b108c9fd1f7..f049050339856 100644 --- a/hphp/hack/test/fanout/modules/module-remove-decl.php +++ b/hphp/hack/test/fanout/modules/module-remove-decl.php @@ -37,11 +37,11 @@ > module A; - +module newtype Bar = Foo; internal newtype Foo = int; //// changed-c-use.php > module A; - +module newtype Bar = Foo; internal newtype Foo = int; diff --git a/hphp/hack/test/fanout/modules/module-remove-decl.php.incr_no_old_decls.exp b/hphp/hack/test/fanout/modules/module-remove-decl.php.incr_no_old_decls.exp index ec9dbe1215eaa..689f7dc3c12c7 100644 --- a/hphp/hack/test/fanout/modules/module-remove-decl.php.incr_no_old_decls.exp +++ b/hphp/hack/test/fanout/modules/module-remove-decl.php.incr_no_old_decls.exp @@ -6,5 +6,6 @@ b-use.php:3:8,8: Unbound name: `A` (a module) (Naming[2049]) c-use.php:3:8,8: Unbound name: `A` (a module) (Naming[2049]) === fanout === Fun foobar +Type Bar Type Foo Type FooA diff --git a/hphp/hack/test/fanout/modules/module-remove-decl.php.incr_old_decl.exp b/hphp/hack/test/fanout/modules/module-remove-decl.php.incr_old_decl.exp index ec9dbe1215eaa..689f7dc3c12c7 100644 --- a/hphp/hack/test/fanout/modules/module-remove-decl.php.incr_old_decl.exp +++ b/hphp/hack/test/fanout/modules/module-remove-decl.php.incr_old_decl.exp @@ -6,5 +6,6 @@ b-use.php:3:8,8: Unbound name: `A` (a module) (Naming[2049]) c-use.php:3:8,8: Unbound name: `A` (a module) (Naming[2049]) === fanout === Fun foobar +Type Bar Type Foo Type FooA diff --git a/hphp/hack/test/fanout/modules/module-remove-decl.php.saved_state.exp b/hphp/hack/test/fanout/modules/module-remove-decl.php.saved_state.exp index ec9dbe1215eaa..689f7dc3c12c7 100644 --- a/hphp/hack/test/fanout/modules/module-remove-decl.php.saved_state.exp +++ b/hphp/hack/test/fanout/modules/module-remove-decl.php.saved_state.exp @@ -6,5 +6,6 @@ b-use.php:3:8,8: Unbound name: `A` (a module) (Naming[2049]) c-use.php:3:8,8: Unbound name: `A` (a module) (Naming[2049]) === fanout === Fun foobar +Type Bar Type Foo Type FooA diff --git a/hphp/hack/test/typecheck/modules/module_newtype.php b/hphp/hack/test/typecheck/modules/module_newtype.php new file mode 100644 index 0000000000000..667ec5f16579e --- /dev/null +++ b/hphp/hack/test/typecheck/modules/module_newtype.php @@ -0,0 +1,33 @@ +//// module.php +> +new module foo {} + +//// decl.php +> +module foo; +module newtype Foo = FooInternal; // ok +module newtype FooBad as FooInternal = FooInternal; // error +internal class FooInternal { + public function foo(): void {} + +} +function same_file(Foo $x) : void { + $x->foo(); // ok +} + +//// use.php +> +module foo; +function same_module(Foo $x) : void { + $x->foo(); // ok, it's FooInternal +} + + +//// use_bad.php +foo(); +} diff --git a/hphp/hack/test/typecheck/modules/module_newtype.php.exp b/hphp/hack/test/typecheck/modules/module_newtype.php.exp new file mode 100644 index 0000000000000..eae9d34d55fb0 --- /dev/null +++ b/hphp/hack/test/typecheck/modules/module_newtype.php.exp @@ -0,0 +1,8 @@ +File "module_newtype.php--decl.php", line 5, characters 26-36: +You cannot use this type in a public declaration. (Typing[4446]) + File "module_newtype.php--decl.php", line 6, characters 16-26: + It is declared as `internal` here +File "module_newtype.php--use_bad.php", line 3, characters 7-9: +You are trying to access the method `foo` but this is a mixed value. Use a **specific** class or interface name. (Typing[4064]) + File "module_newtype.php--use_bad.php", line 2, characters 14-16: + Definition is here diff --git a/hphp/test/slow/modules/module_newtype.php b/hphp/test/slow/modules/module_newtype.php new file mode 100644 index 0000000000000..7981e4960c8fc --- /dev/null +++ b/hphp/test/slow/modules/module_newtype.php @@ -0,0 +1,25 @@ +> +module foo; +module newtype Foo = FooInternal; // ok +internal class FooInternal { + public function foo(): void { + echo "In function foo\n"; + } + +} +function same_file(Foo $x) : void { + $x->foo(); // ok +} + +<<__EntryPoint>> +function main(): void { + include "module_newtype_module.inc"; + include "module_newtype_separate_file.inc"; + include "module_newtype_outside_module.inc"; + + $x = new FooInternal(); + same_file($x); // always ok + separate_file($x); // always ok + outside_module($x); // fails typechecker, but should pass at runtime +} diff --git a/hphp/test/slow/modules/module_newtype.php.expectf b/hphp/test/slow/modules/module_newtype.php.expectf new file mode 100644 index 0000000000000..924c84a48148b --- /dev/null +++ b/hphp/test/slow/modules/module_newtype.php.expectf @@ -0,0 +1,3 @@ +In function foo +In function foo +In function foo diff --git a/hphp/test/slow/modules/module_newtype_module.inc b/hphp/test/slow/modules/module_newtype_module.inc new file mode 100644 index 0000000000000..69e791fe738ab --- /dev/null +++ b/hphp/test/slow/modules/module_newtype_module.inc @@ -0,0 +1,3 @@ +> +new module foo {} diff --git a/hphp/test/slow/modules/module_newtype_outside_module.inc b/hphp/test/slow/modules/module_newtype_outside_module.inc new file mode 100644 index 0000000000000..5cc8185ce6e98 --- /dev/null +++ b/hphp/test/slow/modules/module_newtype_outside_module.inc @@ -0,0 +1,5 @@ +> +function outside_module(Foo $x) : void { + $x->foo(); // ok +} diff --git a/hphp/test/slow/modules/module_newtype_separate_file.inc b/hphp/test/slow/modules/module_newtype_separate_file.inc new file mode 100644 index 0000000000000..0d27f69d4a496 --- /dev/null +++ b/hphp/test/slow/modules/module_newtype_separate_file.inc @@ -0,0 +1,6 @@ +> +module foo; +function separate_file(Foo $x) : void { + $x->foo(); // ok +}