Skip to content

Commit

Permalink
Module Newtype: Correctly declare and lower module newtypes to the ne…
Browse files Browse the repository at this point in the history
…w 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
  • Loading branch information
jamesjwu authored and facebook-github-bot committed Jul 14, 2022
1 parent cf3f1a9 commit 9ebe9e4
Show file tree
Hide file tree
Showing 14 changed files with 96 additions and 4 deletions.
6 changes: 5 additions & 1 deletion hphp/hack/src/decl/direct_decl_smart_constructors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
},
Expand Down
2 changes: 1 addition & 1 deletion hphp/hack/src/hackc/emitter/emit_typedef.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions hphp/hack/src/parser/lowerer/lowerer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5294,6 +5294,7 @@ fn p_def<'a>(node: S<'a>, env: &mut Env<'a>) -> Result<Vec<ast::Def>> {
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)
Expand All @@ -5315,6 +5316,7 @@ fn p_def<'a>(node: S<'a>, env: &mut Env<'a>) -> Result<Vec<ast::Def>> {
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)?,
},
Expand Down
4 changes: 2 additions & 2 deletions hphp/hack/test/fanout/modules/module-remove-decl.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@
<?hh
<<file: __EnableUnstableFeatures('modules')>>
module A;

module newtype Bar = Foo;
internal newtype Foo = int;
//// changed-c-use.php
<?hh
<<file: __EnableUnstableFeatures('modules')>>
module A;

module newtype Bar = Foo;
internal newtype Foo = int;
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
33 changes: 33 additions & 0 deletions hphp/hack/test/typecheck/modules/module_newtype.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//// module.php
<?hh
<<file:__EnableUnstableFeatures("modules")>>
new module foo {}

//// decl.php
<?hh
<<file:__EnableUnstableFeatures("modules")>>
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
<?hh
<<file:__EnableUnstableFeatures("modules")>>
module foo;
function same_module(Foo $x) : void {
$x->foo(); // ok, it's FooInternal
}


//// use_bad.php
<?hh
function bad(Foo $x): void {
$x->foo();
}
8 changes: 8 additions & 0 deletions hphp/hack/test/typecheck/modules/module_newtype.php.exp
Original file line number Diff line number Diff line change
@@ -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
25 changes: 25 additions & 0 deletions hphp/test/slow/modules/module_newtype.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?hh
<<file:__EnableUnstableFeatures("modules")>>
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
}
3 changes: 3 additions & 0 deletions hphp/test/slow/modules/module_newtype.php.expectf
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
In function foo
In function foo
In function foo
3 changes: 3 additions & 0 deletions hphp/test/slow/modules/module_newtype_module.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?hh
<<file:__EnableUnstableFeatures("modules")>>
new module foo {}
5 changes: 5 additions & 0 deletions hphp/test/slow/modules/module_newtype_outside_module.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?hh
<<file:__EnableUnstableFeatures("modules")>>
function outside_module(Foo $x) : void {
$x->foo(); // ok
}
6 changes: 6 additions & 0 deletions hphp/test/slow/modules/module_newtype_separate_file.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?hh
<<file:__EnableUnstableFeatures("modules")>>
module foo;
function separate_file(Foo $x) : void {
$x->foo(); // ok
}

0 comments on commit 9ebe9e4

Please sign in to comment.