From 9c36ce5fd386123744079db65e7e9b22f356a16a Mon Sep 17 00:00:00 2001 From: Chris Rybicki Date: Mon, 5 Aug 2024 17:19:04 -0400 Subject: [PATCH] feat: internal access modifier (#6980) Closes #4156 It's been a while since I've worked with the symbol lookup subsystem. This area is in dire need of refactoring, unfortunately the system is a little too broad to take on in this PR (I'm also not sure what the immediate fix is off the top of my head). Some of the issues: - It's a bit confusing which `lookup_xxx` method should be called in different scenarios - The logic for handling on access modifiers errors is split between two places (once inside `type_check.rs` for determining if field/method accesses are valid, and once inside `symbol_env.rs` for determining if looking up types within environments are valid) - The logic also needs to be added/maintained separately for the LSP. There are also some edge cases I intentionally avoided getting side tracked with to focus on just getting the initial feature working. For example, there might be cases we should look towards other languages for how to handle, like in what situations is it okay to subclass a method as internal. I did some manual QA to validate that autocompletions work as expected with the language server. ## Checklist - [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted) - [x] Description explains motivation and solution - [x] Tests added (always) - [ ] Docs updated (only required for features) - [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing *By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*. --- .../vscode-wing/syntaxes/wing.tmLanguage.json | 2 +- apps/wing/src/commands/pack.test.ts | 7 + docs/api/04-standard-library/std/resource.md | 2 +- .../04-standard-library/ui/api-reference.md | 279 +----------------- .../invalid/internal_access_modifiers.test.w | 23 ++ examples/tests/invalid/package.json | 5 +- examples/wing-fixture/enums.w | 5 + examples/wing-fixture/store.test.w | 26 +- examples/wing-fixture/store.w | 25 ++ libs/wingc/src/ast.rs | 2 + libs/wingc/src/jsify.rs | 4 +- libs/wingc/src/lib.rs | 14 +- libs/wingc/src/lsp/completions.rs | 54 +++- libs/wingc/src/lsp/sync.rs | 9 + libs/wingc/src/parser.rs | 1 + libs/wingc/src/type_check.rs | 209 +++++++++---- libs/wingc/src/type_check/jsii_importer.rs | 62 +++- libs/wingc/src/type_check/symbol_env.rs | 68 ++++- pnpm-lock.yaml | 3 + tools/hangar/__snapshots__/invalid.ts.snap | 104 +++++++ ...ring_wing_library.test.w_compile_tf-aws.md | 81 ++++- 21 files changed, 607 insertions(+), 378 deletions(-) create mode 100644 examples/tests/invalid/internal_access_modifiers.test.w diff --git a/apps/vscode-wing/syntaxes/wing.tmLanguage.json b/apps/vscode-wing/syntaxes/wing.tmLanguage.json index 7a8a94f04de..610de4f1a9a 100644 --- a/apps/vscode-wing/syntaxes/wing.tmLanguage.json +++ b/apps/vscode-wing/syntaxes/wing.tmLanguage.json @@ -128,7 +128,7 @@ "patterns": [ { "name": "keyword.control.flow.wing", - "match": "\\b(else|elif|if|return|throw|try|catch|finally|bring|as)\\b" + "match": "\\b(else|if|return|throw|try|catch|finally|bring|as)\\b" }, { "name": "keyword.control.loop.wing", diff --git a/apps/wing/src/commands/pack.test.ts b/apps/wing/src/commands/pack.test.ts index 6834f60ecf7..610b4d045c1 100644 --- a/apps/wing/src/commands/pack.test.ts +++ b/apps/wing/src/commands/pack.test.ts @@ -210,6 +210,9 @@ describe("wing pack", () => { [ "$preflightTypesMap", "FavoriteNumbers", + "FavoritePlanets", + "InternalClass", + "PublicClass", "Store", "default", "subdir", @@ -234,6 +237,10 @@ describe("wing pack", () => { expect(Object.keys(tarballContents).sort()).toMatchInlineSnapshot(` [ + "$lib/.wing/inflight.InternalClass-2.cjs", + "$lib/.wing/inflight.InternalClass-2.cjs.map", + "$lib/.wing/inflight.PublicClass-2.cjs", + "$lib/.wing/inflight.PublicClass-2.cjs.map", "$lib/.wing/inflight.Store-2.cjs", "$lib/.wing/inflight.Store-2.cjs.map", "$lib/.wing/inflight.Util-1.cjs", diff --git a/docs/api/04-standard-library/std/resource.md b/docs/api/04-standard-library/std/resource.md index eb68c46b0ae..2399a81f9df 100644 --- a/docs/api/04-standard-library/std/resource.md +++ b/docs/api/04-standard-library/std/resource.md @@ -120,7 +120,7 @@ Operations to lift on the object. - *Extends:* ILiftable -- *Implemented By:* BucketRef, Domain, FunctionRef, QueueRef, SecretRef, Api, Bucket, Counter, Domain, Endpoint, Function, OnDeploy, Queue, Schedule, Secret, Service, Topic, Website, Container, Policy, Resource, State, AutoIdResource, Resource, Test, TestRunner, Button, Field, FileBrowser, HttpClient, Section, Table, ValueField, VisualComponent, IAwsFunction, IAwsInflightHost, IApiEndpointHandler, IBucketEventHandler, IFunctionHandler, IOnDeployHandler, IQueueSetConsumerHandler, IScheduleOnTickHandler, IServiceHandler, IServiceStopHandler, ITopicOnMessageHandler, IResourceFactory, ISimulatorInflightHost, ISimulatorResource, IHostedLiftable, IInflight, IInflightHost, IResource, ITestHandler, IButtonHandler, IFieldHandler, IFileBrowserDeleteHandler, IFileBrowserGetHandler, IFileBrowserListHandler, IFileBrowserPutHandler, IHttpClientGetApiSpecHandler, IHttpClientGetUrlHandler, ITableDeleteHandler, ITableGetHandler, ITablePrimaryKeyHandler, ITableScanHandler, ITableUpdateHandler, ITableputHandler, IPredicateHandler +- *Implemented By:* BucketRef, Domain, FunctionRef, QueueRef, SecretRef, Api, Bucket, Counter, Domain, Endpoint, Function, OnDeploy, Queue, Schedule, Secret, Service, Topic, Website, Container, Policy, Resource, State, AutoIdResource, Resource, Test, TestRunner, Button, Field, FileBrowser, HttpClient, Section, Table, ValueField, VisualComponent, IAwsFunction, IAwsInflightHost, IApiEndpointHandler, IBucketEventHandler, IFunctionHandler, IOnDeployHandler, IQueueSetConsumerHandler, IScheduleOnTickHandler, IServiceHandler, IServiceStopHandler, ITopicOnMessageHandler, IResourceFactory, ISimulatorInflightHost, ISimulatorResource, IHostedLiftable, IInflight, IInflightHost, IResource, ITestHandler, IButtonHandler, IFieldHandler, IFileBrowserDeleteHandler, IFileBrowserGetHandler, IFileBrowserListHandler, IFileBrowserPutHandler, IHttpClientGetApiSpecHandler, IHttpClientGetUrlHandler, ITableScanHandler, IPredicateHandler A liftable object that needs to be registered on the host as part of the lifting process. diff --git a/docs/api/04-standard-library/ui/api-reference.md b/docs/api/04-standard-library/ui/api-reference.md index 2f08b59a22a..fb65ba29f9a 100644 --- a/docs/api/04-standard-library/ui/api-reference.md +++ b/docs/api/04-standard-library/ui/api-reference.md @@ -729,22 +729,15 @@ A table can be used to browse files. ```wing bring ui; -new ui.Table(label: str, handlers: TableHandlers); +new ui.Table(handlers: TableHandlers); ``` | **Name** | **Type** | **Description** | | --- | --- | --- | -| label | str | *No description.* | | handlers | TableHandlers | *No description.* | --- -##### `label`Required - -- *Type:* str - ---- - ##### `handlers`Required - *Type:* TableHandlers @@ -1253,60 +1246,7 @@ let TableHandlers = ui.TableHandlers{ ... }; | **Name** | **Type** | **Description** | | --- | --- | --- | -| delete | ITableDeleteHandler | Handler for deleting a row. | -| get | ITableGetHandler | Handler for getting a row. | -| primaryKey | ITablePrimaryKeyHandler | Handler for getting the primary key. | -| put | ITableputHandler | Handler for putting a row. | | scan | ITableScanHandler | Handler for scanning rows. | -| update | ITableUpdateHandler | Handler for updatete a row. | - ---- - -##### `delete`Required - -```wing -delete: ITableDeleteHandler; -``` - -- *Type:* ITableDeleteHandler - -Handler for deleting a row. - ---- - -##### `get`Required - -```wing -get: ITableGetHandler; -``` - -- *Type:* ITableGetHandler - -Handler for getting a row. - ---- - -##### `primaryKey`Required - -```wing -primaryKey: ITablePrimaryKeyHandler; -``` - -- *Type:* ITablePrimaryKeyHandler - -Handler for getting the primary key. - ---- - -##### `put`Required - -```wing -put: ITableputHandler; -``` - -- *Type:* ITableputHandler - -Handler for putting a row. --- @@ -1322,18 +1262,6 @@ Handler for scanning rows. --- -##### `update`Required - -```wing -update: ITableUpdateHandler; -``` - -- *Type:* ITableUpdateHandler - -Handler for updatete a row. - ---- - ## Protocols ### IButtonHandler @@ -1640,164 +1568,6 @@ inflight handle(): str Function that returns the URL to make a request to. -### ITableDeleteHandler - -- *Extends:* IInflight - -- *Implemented By:* ITableDeleteHandler - -**Inflight client:** [@winglang/sdk.ui.ITableDeleteHandlerClient](#@winglang/sdk.ui.ITableDeleteHandlerClient) - -A resource with an inflight "handle" method that can be passed to `ITable`. - - - -### ITableDeleteHandlerClient - -- *Implemented By:* ITableDeleteHandlerClient - -Inflight client for `ITableDeleteHandler`. - -#### Methods - -| **Name** | **Description** | -| --- | --- | -| handle | Function that performs an action. | - ---- - -##### `handle` - -```wing -inflight handle(key: str): void -``` - -Function that performs an action. - -###### `key`Required - -- *Type:* str - ---- - - -### ITableGetHandler - -- *Extends:* IInflight - -- *Implemented By:* ITableGetHandler - -**Inflight client:** [@winglang/sdk.ui.ITableGetHandlerClient](#@winglang/sdk.ui.ITableGetHandlerClient) - -A resource with an inflight "handle" method that can be passed to `ITable`. - - - -### ITableGetHandlerClient - -- *Implemented By:* ITableGetHandlerClient - -Inflight client for `ITableGetHandler`. - -#### Methods - -| **Name** | **Description** | -| --- | --- | -| handle | Function that performs an action. | - ---- - -##### `handle` - -```wing -inflight handle(key: str): Json -``` - -Function that performs an action. - -###### `key`Required - -- *Type:* str - ---- - - -### ITablePrimaryKeyHandler - -- *Extends:* IInflight - -- *Implemented By:* ITablePrimaryKeyHandler - -**Inflight client:** [@winglang/sdk.ui.ITablePrimaryKeyHandlerClient](#@winglang/sdk.ui.ITablePrimaryKeyHandlerClient) - -A resource with an inflight "handle" method that can be passed to `ITable`. - - - -### ITablePrimaryKeyHandlerClient - -- *Implemented By:* ITablePrimaryKeyHandlerClient - -Inflight client for `ITablePrimaryKeyHandler`. - -#### Methods - -| **Name** | **Description** | -| --- | --- | -| handle | Function that performs an action. | - ---- - -##### `handle` - -```wing -inflight handle(): str -``` - -Function that performs an action. - - -### ITableputHandler - -- *Extends:* IInflight - -- *Implemented By:* ITableputHandler - -**Inflight client:** [@winglang/sdk.ui.ITablePutHandlerClient](#@winglang/sdk.ui.ITablePutHandlerClient) - -A resource with an inflight "handle" method that can be passed to `ITable`. - - - -### ITablePutHandlerClient - -- *Implemented By:* ITablePutHandlerClient - -Inflight client for `ITableVoidHandler`. - -#### Methods - -| **Name** | **Description** | -| --- | --- | -| handle | Function that performs an action. | - ---- - -##### `handle` - -```wing -inflight handle(item: str): void -``` - -Function that performs an action. - -###### `item`Required - -- *Type:* str - ---- - - ### ITableScanHandler - *Extends:* IInflight @@ -1833,50 +1603,3 @@ inflight handle(): MutArray Function that performs an action. -### ITableUpdateHandler - -- *Extends:* IInflight - -- *Implemented By:* ITableUpdateHandler - -**Inflight client:** [@winglang/sdk.ui.ITableUpdateHandlerClient](#@winglang/sdk.ui.ITableUpdateHandlerClient) - -A resource with an inflight "handle" method that can be passed to `ITable`. - - - -### ITableUpdateHandlerClient - -- *Implemented By:* ITableUpdateHandlerClient - -Inflight client for `ITableVoidHandler`. - -#### Methods - -| **Name** | **Description** | -| --- | --- | -| handle | Function that performs an action. | - ---- - -##### `handle` - -```wing -inflight handle(key: str, item: str): void -``` - -Function that performs an action. - -###### `key`Required - -- *Type:* str - ---- - -###### `item`Required - -- *Type:* str - ---- - - diff --git a/examples/tests/invalid/internal_access_modifiers.test.w b/examples/tests/invalid/internal_access_modifiers.test.w new file mode 100644 index 00000000000..0c72acaa340 --- /dev/null +++ b/examples/tests/invalid/internal_access_modifiers.test.w @@ -0,0 +1,23 @@ +bring "@winglibs/testfixture" as fixture; + +let e = fixture.FavoritePlanets.MARS; +// ^ Error: Cannot access internal enum 'FavoritePlanets' + +class FakeClass impl fixture.InternalInterface {} +// ^ Error: Cannot access internal interface 'InternalInterface' + +let i = new fixture.InternalClass(); +// ^ Error: Cannot access internal class 'InternalClass' + +fixture.PublicClass.internalStaticMethod(); +// ^ Error: Cannot access internal method 'internalStaticMethod' + +let p = new fixture.PublicClass(); +p.internalField; +// ^ Error: Cannot access internal field 'internalField' + +p.internalMethod(); +// ^ Error: Cannot access internal method 'internalMethod' + +let s = fixture.InternalStruct {}; +// ^ Error: Cannot access internal struct 'InternalStruct' diff --git a/examples/tests/invalid/package.json b/examples/tests/invalid/package.json index 1816c1ceec4..26b9480385a 100644 --- a/examples/tests/invalid/package.json +++ b/examples/tests/invalid/package.json @@ -4,9 +4,10 @@ "cdktf": "0.20.7", "constructs": "^10.3.0", "jsii-code-samples": "1.7.0", - "jsii-fixture": "workspace:^" + "jsii-fixture": "workspace:^", + "@winglibs/testfixture": "workspace:^" }, "volta": { "extends": "../../../package.json" } -} \ No newline at end of file +} diff --git a/examples/wing-fixture/enums.w b/examples/wing-fixture/enums.w index 1a43b685a06..d61f6271ee0 100644 --- a/examples/wing-fixture/enums.w +++ b/examples/wing-fixture/enums.w @@ -2,3 +2,8 @@ pub enum FavoriteNumbers { SEVEN, FORTY_TWO, } + +internal enum FavoritePlanets { + MARS, + JUPITER, +} diff --git a/examples/wing-fixture/store.test.w b/examples/wing-fixture/store.test.w index aaac977506e..94bddea4556 100644 --- a/examples/wing-fixture/store.test.w +++ b/examples/wing-fixture/store.test.w @@ -8,4 +8,28 @@ s.onSet(inflight (message) => { test "onSet" { s.set("hi"); -} \ No newline at end of file +} + +// this is ok since we are in the same package +class FakeClass impl store.InternalInterface, store.PublicInterface {} + +class FakeClass2 extends store.PublicClass { + new() { + this.publicMethod(); + this.internalMethod(); + } +} +class FakeClass3 extends store.InternalClass {} + +new store.PublicClass(); +new store.InternalClass(); +new FakeClass(); + +store.PublicClass.internalStaticMethod(); +store.InternalClass.internalStaticMethod(); + +struct FakeStruct extends store.InternalStruct {} + +let x1 = store.PublicStruct {}; +let x2 = store.InternalStruct {}; +let x3 = FakeStruct {}; diff --git a/examples/wing-fixture/store.w b/examples/wing-fixture/store.w index 5ad06aeb964..79cb6e37a05 100644 --- a/examples/wing-fixture/store.w +++ b/examples/wing-fixture/store.w @@ -28,3 +28,28 @@ pub class Store { this.handlers.push(handler); } } + +pub interface PublicInterface {} + +internal interface InternalInterface {} + +internal class InternalClass { + static internal internalStaticMethod() {} +} + +pub class PublicClass impl PublicInterface, InternalInterface { + static internal internalStaticMethod() {} + pub publicField: num; + internal internalField: num; + + new() { + this.publicField = 42; + this.internalField = 42; + } + + pub publicMethod() {} + internal internalMethod() {} +} + +pub struct PublicStruct {} +internal struct InternalStruct {} diff --git a/libs/wingc/src/ast.rs b/libs/wingc/src/ast.rs index fdfe39b15f8..a236db016a1 100644 --- a/libs/wingc/src/ast.rs +++ b/libs/wingc/src/ast.rs @@ -551,6 +551,7 @@ pub enum AccessModifier { Private, Public, Protected, + Internal, } impl Display for AccessModifier { @@ -559,6 +560,7 @@ impl Display for AccessModifier { AccessModifier::Private => write!(f, "private"), AccessModifier::Public => write!(f, "public"), AccessModifier::Protected => write!(f, "protected"), + AccessModifier::Internal => write!(f, "internal"), } } } diff --git a/libs/wingc/src/jsify.rs b/libs/wingc/src/jsify.rs index a6dde609df3..5bf31342cd4 100644 --- a/libs/wingc/src/jsify.rs +++ b/libs/wingc/src/jsify.rs @@ -2357,7 +2357,7 @@ fn get_exported_symbols(scope: &Scope) -> Vec { StmtKind::Assignment { .. } => {} StmtKind::Scope(_) => {} StmtKind::Class(class) => { - if class.access == AccessModifier::Public { + if class.access == AccessModifier::Public || class.access == AccessModifier::Internal { symbols.push(class.name.clone()); } } @@ -2367,7 +2367,7 @@ fn get_exported_symbols(scope: &Scope) -> Vec { // unless a static method is called on them StmtKind::Struct(_) => {} StmtKind::Enum(enu) => { - if enu.access == AccessModifier::Public { + if enu.access == AccessModifier::Public || enu.access == AccessModifier::Internal { symbols.push(enu.name.clone()); } } diff --git a/libs/wingc/src/lib.rs b/libs/wingc/src/lib.rs index 457fbaf501d..518464928ca 100644 --- a/libs/wingc/src/lib.rs +++ b/libs/wingc/src/lib.rs @@ -120,7 +120,11 @@ const WINGSDK_TEST_CLASS_NAME: &'static str = "Test"; const WINGSDK_NODE: &'static str = "std.Node"; const WINGSDK_SIM_IRESOURCE: &'static str = "sim.IResource"; -const WINGSDK_SIM_IRESOURCE_FQN: &'static str = formatcp!("{}.{}", WINGSDK_ASSEMBLY_NAME, WINGSDK_SIM_IRESOURCE); +const WINGSDK_SIM_IRESOURCE_FQN: &'static str = formatcp!( + "{assembly}.{iface}", + assembly = WINGSDK_ASSEMBLY_NAME, + iface = WINGSDK_SIM_IRESOURCE +); const CONSTRUCT_BASE_CLASS: &'static str = "constructs.Construct"; const CONSTRUCT_BASE_INTERFACE: &'static str = "constructs.IConstruct"; @@ -251,7 +255,13 @@ pub fn type_check( jsii_types: &mut TypeSystem, jsii_imports: &mut Vec, ) { - let mut env = types.add_symbol_env(SymbolEnv::new(None, SymbolEnvKind::Scope, Phase::Preflight, 0)); + let mut env = types.add_symbol_env(SymbolEnv::new( + None, + SymbolEnvKind::Scope, + Phase::Preflight, + 0, + file.package.clone(), + )); types.set_scope_env(scope, env); diff --git a/libs/wingc/src/lsp/completions.rs b/libs/wingc/src/lsp/completions.rs index f5dcf703ca6..9923890adcc 100644 --- a/libs/wingc/src/lsp/completions.rs +++ b/libs/wingc/src/lsp/completions.rs @@ -111,6 +111,7 @@ pub fn on_completion(params: lsp_types::CompletionParams) -> CompletionResponse let file = check_utf8(uri.to_file_path().expect("LSP only works on real filesystems")); let root_ts_node = project_data.trees.get(&file).expect("tree not found").root_node(); let root_scope = project_data.asts.get(&file).expect("ast not found"); + let source_package = project_data.find_source_package(&file); let true_point = Point::new( params.text_document_position.position.line as usize, @@ -235,8 +236,13 @@ pub fn on_completion(params: lsp_types::CompletionParams) -> CompletionResponse ObjectAccessContext::Outside }; - let mut completions = - get_completions_from_type(&nearest_expr_type, &types, Some(found_env.phase), &access_context); + let mut completions = get_completions_from_type( + &nearest_expr_type, + &types, + Some(found_env.phase), + &access_context, + source_package, + ); if nearest_expr_type.is_option() { // check to see if we need to add a ? to the completion let replace_node = if node_to_complete_kind == "." { @@ -317,9 +323,13 @@ pub fn on_completion(params: lsp_types::CompletionParams) -> CompletionResponse .ok() { let completions = match lookup_thing { - SymbolKind::Type(t) => { - get_completions_from_type(&t, &types, Some(found_env.phase), &ObjectAccessContext::Static) - } + SymbolKind::Type(t) => get_completions_from_type( + &t, + &types, + Some(found_env.phase), + &ObjectAccessContext::Static, + source_package, + ), SymbolKind::Variable(v) => { // Handle cases where the parser incorrectly thinks we're in a type reference // This often happens before return statements when returning an anonymous struct literal: @@ -337,6 +347,7 @@ pub fn on_completion(params: lsp_types::CompletionParams) -> CompletionResponse &types, Some(found_env.phase), &str_to_access_context(&reference_text), + source_package, ); } } @@ -344,7 +355,13 @@ pub fn on_completion(params: lsp_types::CompletionParams) -> CompletionResponse } } - get_completions_from_type(&v.type_, &types, Some(found_env.phase), &ObjectAccessContext::Static) + get_completions_from_type( + &v.type_, + &types, + Some(found_env.phase), + &ObjectAccessContext::Static, + source_package, + ) } SymbolKind::Namespace(n) => { // In case the types in this namespace aren't loaded yet, load them now to get completions @@ -360,7 +377,7 @@ pub fn on_completion(params: lsp_types::CompletionParams) -> CompletionResponse // Import all types in the namespace by trying to load the "dummy type" import_udt_from_jsii(&mut types, &jsii_types, &udt, &project_data.jsii_imports); }); - get_completions_from_namespace(&n, Some(found_env.phase)) + get_completions_from_namespace(&n, Some(found_env.phase), source_package) } }; @@ -917,14 +934,15 @@ fn get_completions_from_type( types: &Types, current_phase: Option, access_context: &ObjectAccessContext, + current_package: &str, ) -> Vec { let type_ = *type_.maybe_unwrap_option(); let type_ = types.maybe_unwrap_inference(type_); match &*type_ { - Type::Class(c) => get_completions_from_class(c, current_phase, access_context), - Type::Interface(i) => get_completions_from_class(i, current_phase, access_context), + Type::Class(c) => get_completions_from_class(c, current_phase, access_context, current_package), + Type::Interface(i) => get_completions_from_class(i, current_phase, access_context, current_package), Type::Struct(s) if !matches!(access_context, ObjectAccessContext::Static) => { - get_completions_from_class(s, current_phase, access_context) + get_completions_from_class(s, current_phase, access_context, current_package) } Type::Enum(enum_) => { let variants = &enum_.values; @@ -938,12 +956,12 @@ fn get_completions_from_type( }) .collect() } - Type::Optional(t) => get_completions_from_type(t, types, current_phase, access_context), + Type::Optional(t) => get_completions_from_type(t, types, current_phase, access_context, current_package), Type::Void | Type::Function(_) | Type::Anything | Type::Unresolved | Type::Inferred(_) => vec![], _ => { if let Some(lookup) = types.get_std_class(&type_) { let class = lookup.0.as_type_ref().unwrap().as_class().unwrap(); - get_completions_from_class(class, current_phase, access_context) + get_completions_from_class(class, current_phase, access_context, current_package) } else { vec![] } @@ -954,6 +972,7 @@ fn get_completions_from_type( fn get_completions_from_namespace( namespace: &UnsafeRef, current_phase: Option, + current_package: &str, ) -> Vec { // If a namespace has a class named "Util", then its members can be accessed directly from // the namespace as a syntactic sugar. e.g. "foo.bar()" is equivalent to "foo.Util.bar()" @@ -967,6 +986,7 @@ fn get_completions_from_namespace( util_class, current_phase, &ObjectAccessContext::Static, + current_package, )); } } @@ -977,7 +997,10 @@ fn get_completions_from_namespace( .envs .iter() .flat_map(|env| env.symbol_map.iter()) - .filter(|t| matches!(t.1.access, AccessModifier::Public)) + .filter(|t| { + matches!(t.1.access, AccessModifier::Public) + || (matches!(t.1.access, AccessModifier::Internal) && namespace.source_package == current_package) + }) .flat_map(|(name, symbol)| format_symbol_kind_as_completion(name, &symbol.kind)) .chain(util_completions) .collect() @@ -988,7 +1011,9 @@ fn get_completions_from_class( class: &impl ClassLike, current_phase: Option, access_context: &ObjectAccessContext, + current_package: &str, ) -> Vec { + let allow_internal_access = current_package == class.get_env().source_package; class .get_env() .iter(true) @@ -1005,6 +1030,9 @@ fn get_completions_from_class( if matches!(variable.access, AccessModifier::Private | AccessModifier::Protected) { return None; } + if matches!(variable.access, AccessModifier::Internal) && !allow_internal_access { + return None; + } } // hide private members when accessing from inside the class with "super" ObjectAccessContext::Super => { diff --git a/libs/wingc/src/lsp/sync.rs b/libs/wingc/src/lsp/sync.rs index da568dec9ab..725ab9d0c10 100644 --- a/libs/wingc/src/lsp/sync.rs +++ b/libs/wingc/src/lsp/sync.rs @@ -49,6 +49,15 @@ impl ProjectData { jsii_imports: Vec::new(), } } + + pub fn find_source_package(&self, path: &Utf8Path) -> &str { + self + .file_graph + .iter_files() + .find(|file| file.path == *path) + .map(|file| file.package.as_str()) + .unwrap_or(DEFAULT_PACKAGE_NAME) + } } thread_local! { diff --git a/libs/wingc/src/parser.rs b/libs/wingc/src/parser.rs index f47dd29ebf3..eb5a1f102eb 100644 --- a/libs/wingc/src/parser.rs +++ b/libs/wingc/src/parser.rs @@ -1944,6 +1944,7 @@ impl<'s> Parser<'s> { Some(access_modifier) => match self.node_text(access_modifier) { "pub" => AccessModifier::Public, "protected" => AccessModifier::Protected, + "internal" => AccessModifier::Internal, other => panic!("Unexpected access modifier {}", other), }, None => AccessModifier::Private, diff --git a/libs/wingc/src/type_check.rs b/libs/wingc/src/type_check.rs index 7ad90906648..f96d500b3a7 100644 --- a/libs/wingc/src/type_check.rs +++ b/libs/wingc/src/type_check.rs @@ -27,14 +27,14 @@ use crate::visit_context::{VisitContext, VisitorWithContext}; use crate::visit_stmt_before_super::{CheckSuperCtorLocationVisitor, CheckValidBeforeSuperVisitor}; use crate::visit_types::{VisitType, VisitTypeMut}; use crate::{ - debug, CONSTRUCT_BASE_CLASS, CONSTRUCT_BASE_INTERFACE, CONSTRUCT_NODE_PROPERTY, UTIL_CLASS_NAME, WINGSDK_ARRAY, - WINGSDK_ASSEMBLY_NAME, WINGSDK_BRINGABLE_MODULES, WINGSDK_DURATION, WINGSDK_GENERIC, WINGSDK_IRESOURCE, WINGSDK_JSON, - WINGSDK_MAP, WINGSDK_MUT_ARRAY, WINGSDK_MUT_JSON, WINGSDK_MUT_MAP, WINGSDK_MUT_SET, WINGSDK_NODE, WINGSDK_RESOURCE, - WINGSDK_SET, WINGSDK_SIM_IRESOURCE_FQN, WINGSDK_STD_MODULE, WINGSDK_STRING, WINGSDK_STRUCT, + debug, CONSTRUCT_BASE_CLASS, CONSTRUCT_BASE_INTERFACE, CONSTRUCT_NODE_PROPERTY, DEFAULT_PACKAGE_NAME, + UTIL_CLASS_NAME, WINGSDK_ARRAY, WINGSDK_ASSEMBLY_NAME, WINGSDK_BRINGABLE_MODULES, WINGSDK_DURATION, WINGSDK_GENERIC, + WINGSDK_IRESOURCE, WINGSDK_JSON, WINGSDK_MAP, WINGSDK_MUT_ARRAY, WINGSDK_MUT_JSON, WINGSDK_MUT_MAP, WINGSDK_MUT_SET, + WINGSDK_NODE, WINGSDK_RESOURCE, WINGSDK_SET, WINGSDK_SIM_IRESOURCE_FQN, WINGSDK_STD_MODULE, WINGSDK_STRING, + WINGSDK_STRUCT, }; use camino::{Utf8Path, Utf8PathBuf}; use derivative::Derivative; -use duplicate::duplicate_item; use indexmap::IndexMap; use itertools::{izip, Itertools}; use jsii_importer::JsiiImporter; @@ -291,9 +291,15 @@ impl Display for SpannedTypeInfo { pub struct Namespace { pub name: String, + /// Environments storing the symbols in this namespace + /// Why is this a Vec? Because sometimes a namespace is created from a directory of Wing files, + /// in which case each child source file or subdirectory will have its own environment. #[derivative(Debug = "ignore")] pub envs: Vec, + /// The source package of this namespace + pub source_package: String, + /// Where we can resolve this namespace from pub module_path: ResolveSource, } @@ -1452,8 +1458,20 @@ impl Types { inferences: Vec::new(), type_expressions: IndexMap::new(), append_empty_struct_to_arglist: HashSet::new(), - libraries: SymbolEnv::new(None, SymbolEnvKind::Scope, Phase::Preflight, 0), - intrinsics: SymbolEnv::new(None, SymbolEnvKind::Scope, Phase::Independent, 0), + libraries: SymbolEnv::new( + None, + SymbolEnvKind::Scope, + Phase::Preflight, + 0, + DEFAULT_PACKAGE_NAME.to_string(), + ), + intrinsics: SymbolEnv::new( + None, + SymbolEnvKind::Scope, + Phase::Independent, + 0, + DEFAULT_PACKAGE_NAME.to_string(), + ), // 1 based to avoid conflict with imported JSII classes. This isn't strictly needed since brought JSII classes are never accessed // through their unique ID, but still good to avoid confusion. class_counter: 1, @@ -3194,6 +3212,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); }, func_def.signature.phase, self.ctx.current_stmt_idx(), + self.source_file.package.clone(), )); self.add_arguments_to_env(&func_def.signature.parameters, &sig, &mut function_env); @@ -3604,7 +3623,13 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); child_envs.push(*env); } Some(SymbolEnvOrNamespace::Namespace(ns)) => { - let mut new_env = SymbolEnv::new(None, SymbolEnvKind::Scope, Phase::Independent, 0); + let mut new_env = SymbolEnv::new( + None, + SymbolEnvKind::Scope, + Phase::Independent, + 0, + self.source_file.package.clone(), + ); new_env .define( &Symbol::global(child_file.path.file_stem().unwrap().to_string()), @@ -3667,6 +3692,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); let ns = self.types.add_namespace(Namespace { name: source_file.path.file_stem().unwrap().to_string(), envs: child_envs, + source_package: source_file.package.clone(), module_path: ResolveSource::WingFile, }); self @@ -3812,6 +3838,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); let ns = self.types.add_namespace(Namespace { name: path.to_string(), envs: vec![brought_env], + source_package: self.source_file.package.clone(), module_path: ResolveSource::WingFile, }); if let Err(e) = env.define( @@ -3909,7 +3936,13 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); } // Create environment representing this struct, for now it'll be empty just so we can support referencing it - let dummy_env = SymbolEnv::new(None, SymbolEnvKind::Type(self.types.void()), env.phase, 0); + let dummy_env = SymbolEnv::new( + None, + SymbolEnvKind::Type(self.types.void()), + env.phase, + 0, + self.source_file.package.clone(), + ); // Collect types this struct extends let extends_types = extends @@ -3952,6 +3985,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); SymbolEnvKind::Type(self.types.void()), env.phase, self.ctx.current_stmt_idx(), + self.source_file.package.clone(), ); // Collect types this interface extends @@ -4214,6 +4248,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); SymbolEnvKind::Scope, env.phase, stmt.idx, + self.source_file.package.clone(), )); tc.types.set_scope_env(scope, scope_env); tc.inner_scopes.push((scope, tc.ctx.clone())); @@ -4265,6 +4300,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); SymbolEnvKind::Scope, env.phase, self.ctx.current_stmt_idx(), + self.source_file.package.clone(), )); self.types.set_scope_env(try_statements, try_env); self.inner_scopes.push((try_statements, self.ctx.clone())); @@ -4276,6 +4312,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); SymbolEnvKind::Scope, env.phase, self.ctx.current_stmt_idx(), + self.source_file.package.clone(), )); // Add the exception variable to the catch block @@ -4303,6 +4340,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); SymbolEnvKind::Scope, env.phase, self.ctx.current_stmt_idx(), + self.source_file.package.clone(), )); self.types.set_scope_env(finally_statements, finally_env); self.inner_scopes.push((finally_statements, self.ctx.clone())); @@ -4338,6 +4376,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); SymbolEnvKind::Type(struct_type), Phase::Independent, self.ctx.current_stmt_idx(), + self.source_file.package.clone(), ); // Add fields to the struct env @@ -4394,6 +4433,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); SymbolEnvKind::Type(interface_type), env.phase, self.ctx.current_stmt_idx(), + self.source_file.package.clone(), ); // Add methods to the interface env @@ -4481,7 +4521,13 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); self.extract_parent_class(ast_class.parent.as_ref(), ast_class.phase, &ast_class.name, env); // Create environment representing this class, for now it'll be empty just so we can support referencing ourselves from the class definition. - let dummy_env = SymbolEnv::new(None, SymbolEnvKind::Type(self.types.void()), env.phase, stmt.idx); + let dummy_env = SymbolEnv::new( + None, + SymbolEnvKind::Type(self.types.void()), + env.phase, + stmt.idx, + self.source_file.package.clone(), + ); let impl_interfaces = ast_class .implements @@ -4542,7 +4588,13 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); }; // Create a the real class environment to be filled with the class AST types - let mut class_env = SymbolEnv::new(parent_class_env, SymbolEnvKind::Type(class_type), env.phase, stmt.idx); + let mut class_env = SymbolEnv::new( + parent_class_env, + SymbolEnvKind::Type(class_type), + env.phase, + stmt.idx, + self.source_file.package.clone(), + ); // Add fields to the class env for field in ast_class.fields.iter() { @@ -5019,6 +5071,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); SymbolEnvKind::Scope, env.phase, self.ctx.current_stmt_idx(), + self.source_file.package.clone(), )); self.types.set_scope_env(else_scope, else_scope_env); self.inner_scopes.push((else_scope, self.ctx.clone())); @@ -5057,6 +5110,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); SymbolEnvKind::Scope, env.phase, self.ctx.current_stmt_idx(), + self.source_file.package.clone(), )); self.types.set_scope_env(else_scope, else_scope_env); self.inner_scopes.push((else_scope, self.ctx.clone())); @@ -5072,6 +5126,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); SymbolEnvKind::Scope, env.phase, self.ctx.current_stmt_idx(), + self.source_file.package.clone(), )); self.types.set_scope_env(statements, scope_env); @@ -5101,6 +5156,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); SymbolEnvKind::Scope, env.phase, self.ctx.current_stmt_idx(), + self.source_file.package.clone(), )); match scope_env.define( &iterator, @@ -5218,6 +5274,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); SymbolEnvKind::Scope, env.phase, self.ctx.current_stmt_idx(), + self.source_file.package.clone(), )); // Add the variable to if block scope @@ -5246,6 +5303,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); SymbolEnvKind::Scope, env.phase, self.ctx.current_stmt_idx(), + self.source_file.package.clone(), )); self.types.set_scope_env(statements, if_scope_env); self.inner_scopes.push((statements, self.ctx.clone())); @@ -5407,6 +5465,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); }, method_sig.phase, statement_idx, + self.source_file.package.clone(), )); // Prime the method environment with `this` if !method_def.is_static || is_init { @@ -5448,6 +5507,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); SymbolEnvKind::Type(tc.types.void()), method_sig.phase, 0, + self.source_file.package.clone(), )); tc.types .source_file_envs @@ -5755,7 +5815,13 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); types_map.insert(format!("{o}"), (*o, *n)); } - let dummy_env = SymbolEnv::new(None, SymbolEnvKind::Type(original_type), Phase::Independent, 0); + let dummy_env = SymbolEnv::new( + None, + SymbolEnvKind::Type(original_type), + Phase::Independent, + 0, + self.source_file.package.clone(), + ); let tt = match &*original_type { Type::Class(c) => Type::Class(Class { name: c.name.clone(), @@ -5791,8 +5857,14 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); // TODO: here we add a new type regardless whether we already "hydrated" `original_type` with these `type_params`. Cache! let mut new_type = self.types.add_type(tt); - let new_env = - SymbolEnv::new_with_type_params(None, SymbolEnvKind::Type(new_type), Phase::Independent, 0, type_params); + let new_env = SymbolEnv::new_with_type_params( + None, + SymbolEnvKind::Type(new_type), + Phase::Independent, + 0, + self.source_file.package.clone(), + type_params, + ); // Update the types's env to point to the new env match *new_type { @@ -6149,7 +6221,8 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); if symbol.name == "print" { self.spanned_error(symbol, "Unknown symbol \"print\", did you mean to use \"log\"?"); } else { - self.type_error(lookup_result_mut_to_type_error(lookup_res, symbol)); + let lookup_res = env.lookup_ext(symbol, Some(self.ctx.current_stmt_idx())); + self.type_error(lookup_result_to_type_error(lookup_res, symbol)); } ( ResolveReferenceResult::Variable(self.make_error_variable_info()), @@ -6515,24 +6588,27 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); }; // Check if the class in which the property is defined is one of the classes we're currently nested in - let mut private_access = false; - let mut protected_access = false; + let mut allow_private_access = false; + let mut allow_protected_access = false; for current_class in self.ctx.current_class_nesting() { let current_class_type = self .resolve_user_defined_type(¤t_class, env, self.ctx.current_stmt_idx()) .unwrap(); - private_access = private_access || current_class_type.is_same_type_as(&property_defined_in); - protected_access = - protected_access || private_access || current_class_type.is_strict_subtype_of(&property_defined_in); - if private_access { + allow_private_access = allow_private_access || current_class_type.is_same_type_as(&property_defined_in); + allow_protected_access = allow_protected_access + || allow_private_access + || current_class_type.is_strict_subtype_of(&property_defined_in); + if allow_private_access { break; } } + let allow_internal_access = self.source_file.package == lookup_info.env.source_package; + // Compare the access type with what's allowed match var.access { AccessModifier::Private => { - if !private_access { + if !allow_private_access { report_diagnostic(Diagnostic { message: format!("Cannot access private member \"{property}\" of \"{class}\""), span: Some(property.span()), @@ -6548,7 +6624,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); } } AccessModifier::Protected => { - if !protected_access { + if !allow_protected_access { report_diagnostic(Diagnostic { message: format!("Cannot access protected member \"{property}\" of \"{class}\""), span: Some(property.span()), @@ -6563,6 +6639,23 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); }); } } + AccessModifier::Internal => { + let other_package = &lookup_info.env.source_package; + if !allow_internal_access { + report_diagnostic(Diagnostic { + message: format!("Cannot access internal member \"{property}\" of \"{class}\""), + span: Some(property.span()), + annotations: vec![DiagnosticAnnotation { + message: "defined here".to_string(), + span: lookup_info.span, + }], + hints: vec![format!( + "the definition of \"{property}\" needs a broader access modifier like \"pub\" to be used outside of \"{other_package}\"", + )], + severity: DiagnosticSeverity::Error, + }); + } + } AccessModifier::Public => {} // keep this here to make sure we don't add a new access modifier without handling it here } @@ -6720,6 +6813,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] }); SymbolEnvKind::Scope, env.phase, self.ctx.current_stmt_idx(), + self.source_file.package.clone(), )); self.types.set_scope_env(&lift_quals.statements, scope_env); self.inner_scopes.push((&lift_quals.statements, self.ctx.clone())); @@ -6884,11 +6978,6 @@ fn add_parent_members_to_iface_env( Ok(()) } -#[duplicate_item( - lookup_result_to_type_error LookupResult; - [lookup_result_to_type_error] [LookupResult]; - [lookup_result_mut_to_type_error] [LookupResultMut]; -)] fn lookup_result_to_type_error(lookup_result: LookupResult, looked_up_object: &T) -> TypeError where T: Spanned + Display, @@ -6911,34 +7000,46 @@ where hints, } } - LookupResult::NotPublic(kind, lookup_info) => TypeError { - message: { - let access = lookup_info.access.to_string(); - match kind { - SymbolKind::Type(type_) => { - if matches!(**type_, Type::Class(_)) { - format!("Class \"{looked_up_object}\" is {access}") - } else if matches!(**type_, Type::Interface(_)) { - format!("Interface \"{looked_up_object}\" is {access}") - } else if matches!(**type_, Type::Struct(_)) { - format!("Struct \"{looked_up_object}\" is {access}") - } else if matches!(**type_, Type::Enum(_)) { - format!("Enum \"{looked_up_object}\" is {access}") - } else { - format!("Symbol \"{looked_up_object}\" is {access}") + LookupResult::NotPublic(kind, lookup_info) => { + let access = lookup_info.access; + let source_package = &lookup_info.env.source_package; + TypeError { + message: { + let access_str = access.to_string(); + match kind { + SymbolKind::Type(type_) => { + if matches!(**type_, Type::Class(_)) { + format!("Class \"{looked_up_object}\" is {access_str}") + } else if matches!(**type_, Type::Interface(_)) { + format!("Interface \"{looked_up_object}\" is {access_str}") + } else if matches!(**type_, Type::Struct(_)) { + format!("Struct \"{looked_up_object}\" is {access_str}") + } else if matches!(**type_, Type::Enum(_)) { + format!("Enum \"{looked_up_object}\" is {access_str}") + } else { + format!("Symbol \"{looked_up_object}\" is {access_str}") + } } + SymbolKind::Variable(_) => format!("Symbol \"{looked_up_object}\" is {access_str}"), + SymbolKind::Namespace(_) => format!("namespace \"{looked_up_object}\" is {access_str}"), } - SymbolKind::Variable(_) => format!("Symbol \"{looked_up_object}\" is {access}"), - SymbolKind::Namespace(_) => format!("namespace \"{looked_up_object}\" is {access}"), - } - }, - span: looked_up_object.span(), - annotations: vec![DiagnosticAnnotation { - message: "defined here".to_string(), - span: lookup_info.span, - }], - hints: vec![], - }, + }, + span: looked_up_object.span(), + annotations: vec![DiagnosticAnnotation { + message: "defined here".to_string(), + span: lookup_info.span, + }], + hints: { + let mut hints = vec![]; + if access == AccessModifier::Internal { + hints.push(format!( + "the definition of \"{looked_up_object}\" needs a broader access modifier like \"pub\" to be used outside of \"{source_package}\"", + )); + } + hints + }, + } + } LookupResult::MultipleFound => TypeError { message: format!("Ambiguous symbol \"{looked_up_object}\""), span: looked_up_object.span(), diff --git a/libs/wingc/src/type_check/jsii_importer.rs b/libs/wingc/src/type_check/jsii_importer.rs index 9ad3c1ee0db..858488e7f3c 100644 --- a/libs/wingc/src/type_check/jsii_importer.rs +++ b/libs/wingc/src/type_check/jsii_importer.rs @@ -169,12 +169,17 @@ impl<'a> JsiiImporter<'a> { // First, setup the assembly namespace if self.wing_types.libraries.lookup(&assembly, None).is_none() { - let ns_env = self - .wing_types - .add_symbol_env(SymbolEnv::new(None, SymbolEnvKind::Scope, Phase::Preflight, 0)); + let ns_env = self.wing_types.add_symbol_env(SymbolEnv::new( + None, + SymbolEnvKind::Scope, + Phase::Preflight, + 0, + assembly.name.clone(), + )); let ns = self.wing_types.add_namespace(Namespace { name: assembly.to_string(), envs: vec![ns_env], + source_package: assembly.name.clone(), module_path: ResolveSource::ExternalModule(assembly.name.clone()), }); self @@ -206,9 +211,13 @@ impl<'a> JsiiImporter<'a> { .unwrap(); if parent_ns.envs.get_mut(0).unwrap().lookup_mut(&ns_sym, None).is_none() { - let ns_env = self - .wing_types - .add_symbol_env(SymbolEnv::new(None, SymbolEnvKind::Scope, Phase::Preflight, 0)); + let ns_env = self.wing_types.add_symbol_env(SymbolEnv::new( + None, + SymbolEnvKind::Scope, + Phase::Preflight, + 0, + assembly.name.clone(), + )); // Special case for the SDK, we are able to alias the namespace let module_path = if assembly.name == WINGSDK_ASSEMBLY_NAME { format!("{}/{}", lookup_vec.join("/"), namespace_name) @@ -219,6 +228,7 @@ impl<'a> JsiiImporter<'a> { let ns = self.wing_types.add_namespace(Namespace { name: namespace_name.to_string(), envs: vec![ns_env], + source_package: assembly.name.clone(), module_path: ResolveSource::ExternalModule(module_path), }); parent_ns @@ -346,6 +356,7 @@ impl<'a> JsiiImporter<'a> { debug!("Importing interface {}", jsii_interface_fqn.as_str().green()); let type_name = jsii_interface_fqn.type_name(); + let assembly = jsii_interface_fqn.assembly(); let is_struct = match jsii_interface.datatype { Some(true) => { // If this datatype has methods something is unexpected in this JSII type definition, skip it. @@ -381,6 +392,7 @@ impl<'a> JsiiImporter<'a> { SymbolEnvKind::Type(self.wing_types.void()), Phase::Independent, // structs are phase-independent 0, + assembly.to_string(), ), })), false => self.wing_types.add_type(Type::Interface(Interface { @@ -390,7 +402,13 @@ impl<'a> JsiiImporter<'a> { extends: vec![], docs: Docs::from(&jsii_interface.docs), // Will be replaced below - env: SymbolEnv::new(None, SymbolEnvKind::Type(self.wing_types.void()), phase, 0), + env: SymbolEnv::new( + None, + SymbolEnvKind::Type(self.wing_types.void()), + phase, + 0, + assembly.to_string(), + ), phase, })), }; @@ -408,7 +426,7 @@ impl<'a> JsiiImporter<'a> { } }; - let mut iface_env = SymbolEnv::new(None, SymbolEnvKind::Type(wing_type), phase, 0); + let mut iface_env = SymbolEnv::new(None, SymbolEnvKind::Type(wing_type), phase, 0, assembly.to_string()); iface_env.type_parameters = self.type_param_from_docs(&jsii_interface_fqn, &jsii_interface.docs); self.add_members_to_class_env( @@ -677,6 +695,7 @@ impl<'a> JsiiImporter<'a> { let jsii_class_fqn = FQN::from(jsii_class.fqn.as_str()); debug!("Importing class {}", jsii_class_fqn.as_str().green()); let type_name = jsii_class_fqn.type_name(); + let assembly = jsii_class_fqn.assembly(); // Get the base class of the JSII class, define it via recursive call if it's not define yet let base_class_type = if let Some(base_class_fqn) = &jsii_class.base { @@ -734,7 +753,13 @@ impl<'a> JsiiImporter<'a> { }; // Create environment representing this class, for now it'll be empty just so we can support referencing ourselves from the class definition. - let dummy_env = SymbolEnv::new(None, SymbolEnvKind::Type(self.wing_types.void()), class_phase, 0); + let dummy_env = SymbolEnv::new( + None, + SymbolEnvKind::Type(self.wing_types.void()), + class_phase, + 0, + assembly.to_string(), + ); let new_type_symbol = Self::jsii_name_to_symbol(type_name, &jsii_class.location_in_module); // Create the new resource/class type and add it to the current environment. // When adding the class methods below we'll be able to reference this type. @@ -774,7 +799,13 @@ impl<'a> JsiiImporter<'a> { }; // Create class's actual environment before we add properties and methods to it - let mut class_env = SymbolEnv::new(base_class_env, SymbolEnvKind::Type(new_type), class_phase, 0); + let mut class_env = SymbolEnv::new( + base_class_env, + SymbolEnvKind::Type(new_type), + class_phase, + 0, + assembly.to_string(), + ); class_env.type_parameters = self.type_param_from_docs(&jsii_class_fqn, &jsii_class.docs); // Add the class's constructor to the class environment, if the class has one which is public @@ -980,12 +1011,17 @@ impl<'a> JsiiImporter<'a> { .lookup(&assembly.name.as_str().into(), None) .is_none() { - let ns_env = self - .wing_types - .add_symbol_env(SymbolEnv::new(None, SymbolEnvKind::Scope, Phase::Preflight, 0)); + let ns_env = self.wing_types.add_symbol_env(SymbolEnv::new( + None, + SymbolEnvKind::Scope, + Phase::Preflight, + 0, + assembly.name.clone(), + )); let ns = self.wing_types.add_namespace(Namespace { name: assembly.name.clone(), envs: vec![ns_env], + source_package: assembly.name.clone(), module_path: ResolveSource::ExternalModule(assembly.name.clone()), }); self diff --git a/libs/wingc/src/type_check/symbol_env.rs b/libs/wingc/src/type_check/symbol_env.rs index c5a77b20473..591806f3ae8 100644 --- a/libs/wingc/src/type_check/symbol_env.rs +++ b/libs/wingc/src/type_check/symbol_env.rs @@ -33,6 +33,10 @@ pub struct SymbolEnv { pub phase: Phase, pub type_parameters: Option>, statement_idx: usize, + + /// The source code package the environment is associated with + /// (for example, if the environment is for a class, this would be the package the class is defined in) + pub source_package: String, } #[derive(Debug)] @@ -185,7 +189,13 @@ pub struct SymbolLookupInfo { } impl SymbolEnv { - pub fn new(parent: Option, kind: SymbolEnvKind, phase: Phase, statement_idx: usize) -> Self { + pub fn new( + parent: Option, + kind: SymbolEnvKind, + phase: Phase, + statement_idx: usize, + source_package: String, + ) -> Self { assert_is_type_env(parent, &kind); Self { @@ -195,6 +205,7 @@ impl SymbolEnv { phase, statement_idx, type_parameters: None, + source_package, } } @@ -203,6 +214,7 @@ impl SymbolEnv { kind: SymbolEnvKind, phase: Phase, statement_idx: usize, + source_package: String, type_params: Vec>, ) -> Self { assert_is_type_env(parent, &kind); @@ -214,6 +226,7 @@ impl SymbolEnv { phase, statement_idx, type_parameters: Some(type_params), + source_package, } } @@ -351,11 +364,10 @@ impl SymbolEnv { if let Some(ref_annotation([parent_env])) = self.parent { let res = parent_env.lookup_ext(symbol, not_after_stmt_idx.map(|_| self.statement_idx)); // The `NotFound` result needs to include the type of the original (non-parent) environment (if applicable) - let res = match res { + return match res { LookupResult::NotFound(s, ..) => LookupResult::NotFound(s, env_type), _ => res, }; - return res; } LookupResult::NotFound(symbol.clone(), env_type) @@ -373,6 +385,7 @@ impl SymbolEnv { let mut it = nested_vec.iter(); let symb = *it.next().unwrap(); + let caller_source_package = self.source_package.clone(); let res = self.lookup_ext(symb, statement_idx); let mut res = if let LookupResult::Found(k, i) = res { @@ -398,11 +411,25 @@ impl SymbolEnv { return LookupResult::ExpectedNamespace(prev_symb.clone()); }; - // Look up the result in each env. If there are multiple results, throw a special error - // otherwise proceed normally + // Here we're looking up some symbol in each of the environments stored in the Namespace. + // Recall that a Namespace could represent a directory, in which case it points to multiple environments, + // one for each file or subdirectory. + // Suppose the user is trying to access a class named "Foo" inside a folder named "ns1". + // It's possible that both foo/file1.w and foo/file2.w could have a class named "Foo" defined inside. + // So we need to handle these possible conflicts. + // + // The way we have resolve the conflicts currently is we have a variable named "lookup_result" that is + // initialized before a for loop. Inside the for loop, we look up the target symbol in each of the child + // environments, and store it in "partial_result". Then, we compare "partial_result" with "lookup_result". + // and handle various cases. + // + // For example: + // - If "Foo" was found in both foo/file1.w and foo/file2.w, we return "MultipleFound". + // - If private "Foo" was found in foo/file1.w and public "Foo" was found in foo/file2.w, + // we return the public version. let mut lookup_result = LookupResult::NotFound((*next_symb).clone(), None); for env in ns.envs.vec_iter() { - // invariant: lookup_result is never "ExpectedNamespace" or "MultipleFound" + // invariant: lookup_result is never "ExpectedNamespace" or "MultipleFound" (?) // We're looking up a symbol in a namespace other than our own, so we need to // check if the symbol is public or not. If it's not, replace a "Found" result @@ -410,6 +437,14 @@ impl SymbolEnv { let partial_result = match env.lookup_ext(next_symb, statement_idx) { LookupResult::Found(kind, info) => match info.access { AccessModifier::Public => LookupResult::Found(kind, info), + AccessModifier::Internal => { + // If the symbol is internal, we can only use it if it's in the same package + if ns.source_package == caller_source_package { + LookupResult::Found(kind, info) + } else { + LookupResult::NotPublic(kind, info) + } + } AccessModifier::Private => LookupResult::NotPublic(kind, info), AccessModifier::Protected => panic!("symbols in namespaces cannot be protected"), }, @@ -445,6 +480,7 @@ impl SymbolEnv { lookup_result = partial_result; } } + match lookup_result { LookupResult::Found(k, i) => { res = (k, i); @@ -554,6 +590,7 @@ mod tests { symbol_env::{LookupResult, SymbolEnvKind}, Namespace, ResolveSource, SymbolKind, Types, }, + DEFAULT_PACKAGE_NAME, }; use super::{StatementIdx, SymbolEnv}; @@ -565,13 +602,15 @@ mod tests { #[test] fn test_statement_idx_lookups() { let types = setup_types(); - let mut parent_env = SymbolEnv::new(None, SymbolEnvKind::Scope, Phase::Independent, 0); + let source_pkg = "pkg1".to_string(); + let mut parent_env = SymbolEnv::new(None, SymbolEnvKind::Scope, Phase::Independent, 0, source_pkg.clone()); let child_scope_idx = 10; let mut child_env = SymbolEnv::new( Some(parent_env.get_ref()), SymbolEnvKind::Scope, crate::ast::Phase::Independent, child_scope_idx, + source_pkg, ); // Define a globally visible variable in the parent env @@ -687,30 +726,41 @@ mod tests { #[test] fn test_nested_lookups() { let mut types = setup_types(); - let mut parent_env = SymbolEnv::new(None, SymbolEnvKind::Scope, Phase::Independent, 0); + let source_pkg = "pkg1".to_string(); + let mut parent_env = SymbolEnv::new(None, SymbolEnvKind::Scope, Phase::Independent, 0, source_pkg.clone()); let child_env = SymbolEnv::new( Some(parent_env.get_ref()), SymbolEnvKind::Scope, crate::ast::Phase::Independent, 0, + source_pkg.clone(), ); // Create namespaces - let mut ns1_env = types.add_symbol_env(SymbolEnv::new(None, SymbolEnvKind::Scope, Phase::Independent, 0)); + let mut ns1_env = types.add_symbol_env(SymbolEnv::new( + None, + SymbolEnvKind::Scope, + Phase::Independent, + 0, + source_pkg.clone(), + )); let mut ns2_env = types.add_symbol_env(SymbolEnv::new( Some(ns1_env), SymbolEnvKind::Scope, Phase::Independent, 0, + source_pkg, )); let ns1 = types.add_namespace(Namespace { name: "ns1".to_string(), envs: vec![ns1_env], + source_package: DEFAULT_PACKAGE_NAME.to_string(), module_path: ResolveSource::WingFile, }); let ns2 = types.add_namespace(Namespace { name: "ns2".to_string(), envs: vec![ns2_env], + source_package: DEFAULT_PACKAGE_NAME.to_string(), module_path: ResolveSource::WingFile, }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 8306c25ad14..587ed5d7d69 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1033,6 +1033,9 @@ importers: examples/tests/invalid: dependencies: + '@winglibs/testfixture': + specifier: workspace:^ + version: link:../../wing-fixture cdktf: specifier: 0.20.7 version: 0.20.7(constructs@10.3.0) diff --git a/tools/hangar/__snapshots__/invalid.ts.snap b/tools/hangar/__snapshots__/invalid.ts.snap index 6906277f3e5..b49b58442c1 100644 --- a/tools/hangar/__snapshots__/invalid.ts.snap +++ b/tools/hangar/__snapshots__/invalid.ts.snap @@ -2843,6 +2843,110 @@ Test Files 1 failed (1) Duration " `; +exports[`internal_access_modifiers.test.w 1`] = ` +"error: Expected identifier "fixture" to be a variable, but it's a namespace + --> ../../../examples/tests/invalid/internal_access_modifiers.test.w:3:9 + | +3 | let e = fixture.FavoritePlanets.MARS; + | ^^^^^^^ + + +error: Interface "fixture.InternalInterface" is internal + --> ../../../examples/tests/invalid/internal_access_modifiers.test.w:6:22 + | + 6 | class FakeClass impl fixture.InternalInterface {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + --> ../../../examples/tests/invalid/node_modules/@winglibs/testfixture/store.w:34:20 + | +34 | internal interface InternalInterface {} + | ----------------- defined here + | + = hint: the definition of "fixture.InternalInterface" needs a broader access modifier like "pub" to be used outside of "@winglibs/testfixture" + + +error: Expected an interface, instead found type "unresolved" + --> ../../../examples/tests/invalid/internal_access_modifiers.test.w:6:22 + | +6 | class FakeClass impl fixture.InternalInterface {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + + +error: Class "fixture.InternalClass" is internal + --> ../../../examples/tests/invalid/internal_access_modifiers.test.w:9:13 + | + 9 | let i = new fixture.InternalClass(); + | ^^^^^^^^^^^^^^^^^^^^^ + | + --> ../../../examples/tests/invalid/node_modules/@winglibs/testfixture/store.w:36:16 + | +36 | internal class InternalClass { + | ------------- defined here + | + = hint: the definition of "fixture.InternalClass" needs a broader access modifier like "pub" to be used outside of "@winglibs/testfixture" + + +error: Cannot access internal member "internalStaticMethod" of "PublicClass" + --> ../../../examples/tests/invalid/internal_access_modifiers.test.w:12:21 + | +12 | fixture.PublicClass.internalStaticMethod(); + | ^^^^^^^^^^^^^^^^^^^^ + | + --> ../../../examples/tests/invalid/node_modules/@winglibs/testfixture/store.w:41:19 + | +41 | static internal internalStaticMethod() {} + | -------------------- defined here + | + = hint: the definition of "internalStaticMethod" needs a broader access modifier like "pub" to be used outside of "@winglibs/testfixture" + + +error: Cannot access internal member "internalField" of "PublicClass" + --> ../../../examples/tests/invalid/internal_access_modifiers.test.w:16:3 + | +16 | p.internalField; + | ^^^^^^^^^^^^^ + | + --> ../../../examples/tests/invalid/node_modules/@winglibs/testfixture/store.w:43:12 + | +43 | internal internalField: num; + | ------------- defined here + | + = hint: the definition of "internalField" needs a broader access modifier like "pub" to be used outside of "@winglibs/testfixture" + + +error: Cannot access internal member "internalMethod" of "PublicClass" + --> ../../../examples/tests/invalid/internal_access_modifiers.test.w:19:3 + | +19 | p.internalMethod(); + | ^^^^^^^^^^^^^^ + | + --> ../../../examples/tests/invalid/node_modules/@winglibs/testfixture/store.w:51:12 + | +51 | internal internalMethod() {} + | -------------- defined here + | + = hint: the definition of "internalMethod" needs a broader access modifier like "pub" to be used outside of "@winglibs/testfixture" + + +error: Struct "fixture.InternalStruct" is internal + --> ../../../examples/tests/invalid/internal_access_modifiers.test.w:22:9 + | +22 | let s = fixture.InternalStruct {}; + | ^^^^^^^^^^^^^^^^^^^^^^ + | + --> ../../../examples/tests/invalid/node_modules/@winglibs/testfixture/store.w:55:17 + | +55 | internal struct InternalStruct {} + | -------------- defined here + | + = hint: the definition of "fixture.InternalStruct" needs a broader access modifier like "pub" to be used outside of "@winglibs/testfixture" + +Tests 1 failed (1) +Snapshots 1 skipped +Test Files 1 failed (1) +Duration " +`; + exports[`intrinsics.test.w 1`] = ` "error: @dirname does not expect arguments --> ../../../examples/tests/invalid/intrinsics.test.w:6:20 diff --git a/tools/hangar/__snapshots__/test_corpus/valid/bring_wing_library.test.w_compile_tf-aws.md b/tools/hangar/__snapshots__/test_corpus/valid/bring_wing_library.test.w_compile_tf-aws.md index f148d76ce8a..e43a2f38324 100644 --- a/tools/hangar/__snapshots__/test_corpus/valid/bring_wing_library.test.w_compile_tf-aws.md +++ b/tools/hangar/__snapshots__/test_corpus/valid/bring_wing_library.test.w_compile_tf-aws.md @@ -22,6 +22,32 @@ module.exports = function({ $fixture_Store }) { //# sourceMappingURL=inflight.$Closure1-3.cjs.map ``` +## inflight.InternalClass-2.cjs +```cjs +"use strict"; +const $helpers = require("@winglang/sdk/lib/helpers"); +const $macros = require("@winglang/sdk/lib/macros"); +module.exports = function({ }) { + class InternalClass { + } + return InternalClass; +} +//# sourceMappingURL=inflight.InternalClass-2.cjs.map +``` + +## inflight.PublicClass-2.cjs +```cjs +"use strict"; +const $helpers = require("@winglang/sdk/lib/helpers"); +const $macros = require("@winglang/sdk/lib/macros"); +module.exports = function({ }) { + class PublicClass { + } + return PublicClass; +} +//# sourceMappingURL=inflight.PublicClass-2.cjs.map +``` + ## inflight.Store-2.cjs ```cjs "use strict"; @@ -174,7 +200,14 @@ const FavoriteNumbers = return tmp; })({}) ; -module.exports = { $preflightTypesMap, FavoriteNumbers }; +const FavoritePlanets = + (function (tmp) { + tmp["MARS"] = "MARS"; + tmp["JUPITER"] = "JUPITER"; + return tmp; + })({}) +; +module.exports = { $preflightTypesMap, FavoriteNumbers, FavoritePlanets }; //# sourceMappingURL=preflight.enums-1.cjs.map ``` @@ -236,7 +269,51 @@ class Store extends $stdlib.std.Resource { }); } } -module.exports = { $preflightTypesMap, Store }; +class InternalClass extends $stdlib.std.Resource { + constructor($scope, $id, ) { + super($scope, $id); + } + static internalStaticMethod($scope) { + } + static _toInflightType() { + return ` + require("${$helpers.normalPath(__dirname)}/inflight.InternalClass-2.cjs")({ + }) + `; + } + get _liftMap() { + return ({ + "$inflight_init": [ + ], + }); + } +} +class PublicClass extends $stdlib.std.Resource { + constructor($scope, $id, ) { + super($scope, $id); + this.publicField = 42; + this.internalField = 42; + } + static internalStaticMethod($scope) { + } + publicMethod() { + } + internalMethod() { + } + static _toInflightType() { + return ` + require("${$helpers.normalPath(__dirname)}/inflight.PublicClass-2.cjs")({ + }) + `; + } + get _liftMap() { + return ({ + "$inflight_init": [ + ], + }); + } +} +module.exports = { $preflightTypesMap, Store, InternalClass, PublicClass }; //# sourceMappingURL=preflight.store-3.cjs.map ```