From 6ae387a1d1a9648e16acdc3e50cbb1fbed351f79 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Tue, 3 Oct 2023 00:00:21 +0200 Subject: [PATCH] fix: resolution of references to declarations of wrong node type (#599) ### Summary of Changes References to declarations of the wrong node type were incorrectly resolved if they were imported in a qualified import. This PR adds tests to reproduce the bug and the missing check of the node type. --------- Co-authored-by: megalinter-bot <129584137+megalinter-bot@users.noreply.github.com> --- .../scoping/safe-ds-scope-provider.ts | 39 ++++++++++--------- .../main with qualified import.sdstest | 5 ++- .../resource other package.sdstest | 3 ++ .../main with qualified import.sdstest | 7 +++- .../resource other package.sdstest | 3 ++ .../main with qualified import.sdstest | 7 +++- .../resource other package.sdstest | 3 ++ 7 files changed, 44 insertions(+), 23 deletions(-) diff --git a/src/language/scoping/safe-ds-scope-provider.ts b/src/language/scoping/safe-ds-scope-provider.ts index 064cee6c3..34a44f6d7 100644 --- a/src/language/scoping/safe-ds-scope-provider.ts +++ b/src/language/scoping/safe-ds-scope-provider.ts @@ -1,6 +1,7 @@ import { AstNode, AstNodeDescription, + AstReflection, DefaultScopeProvider, EMPTY_SCOPE, getContainerOfType, @@ -62,12 +63,14 @@ import { SafeDsTypeComputer } from '../typing/safe-ds-type-computer.js'; import { SafeDsPackageManager } from '../workspace/safe-ds-package-manager.js'; export class SafeDsScopeProvider extends DefaultScopeProvider { + private readonly astReflection: AstReflection; private readonly packageManager: SafeDsPackageManager; private readonly typeComputer: SafeDsTypeComputer; constructor(services: SafeDsServices) { super(services); + this.astReflection = services.shared.AstReflection; this.packageManager = services.workspace.PackageManager; this.typeComputer = services.types.TypeComputer; } @@ -350,6 +353,23 @@ export class SafeDsScopeProvider extends DefaultScopeProvider { return this.createScope(explicitlyImportedDeclarations, outerScope); } + private builtinDeclarations(referenceType: string): AstNodeDescription[] { + return this.packageManager.getDeclarationsInPackageOrSubpackage('safeds', { + nodeType: referenceType, + hideInternal: true, + }); + } + + private declarationsInSamePackage(packageName: string | null, referenceType: string): AstNodeDescription[] { + if (!packageName) { + return []; + } + + return this.packageManager.getDeclarationsInPackage(packageName, { + nodeType: referenceType, + }); + } + private explicitlyImportedDeclarations(referenceType: string, node: AstNode): AstNodeDescription[] { const containingModule = getContainerOfType(node, isSdsModule); const imports = importsOrEmpty(containingModule); @@ -359,7 +379,7 @@ export class SafeDsScopeProvider extends DefaultScopeProvider { if (isSdsQualifiedImport(imp)) { for (const importedDeclaration of importedDeclarationsOrEmpty(imp)) { const description = importedDeclaration.declaration.$nodeDescription; - if (!description) { + if (!description || !this.astReflection.isSubtype(description.type, referenceType)) { continue; } @@ -380,21 +400,4 @@ export class SafeDsScopeProvider extends DefaultScopeProvider { return result; } - - private declarationsInSamePackage(packageName: string | null, referenceType: string): AstNodeDescription[] { - if (!packageName) { - return []; - } - - return this.packageManager.getDeclarationsInPackage(packageName, { - nodeType: referenceType, - }); - } - - private builtinDeclarations(referenceType: string): AstNodeDescription[] { - return this.packageManager.getDeclarationsInPackageOrSubpackage('safeds', { - nodeType: referenceType, - hideInternal: true, - }); - } } diff --git a/tests/resources/scoping/annotation calls/across files/main with qualified import.sdstest b/tests/resources/scoping/annotation calls/across files/main with qualified import.sdstest index 904dd5731..d6c494b7b 100644 --- a/tests/resources/scoping/annotation calls/across files/main with qualified import.sdstest +++ b/tests/resources/scoping/annotation calls/across files/main with qualified import.sdstest @@ -1,7 +1,7 @@ package tests.scoping.annotationCalls.acrossFiles from safeds.scoping.annotationCalls.acrossFiles import MyAnnotation -from tests.scoping.annotationCalls.acrossFiles.other import AnnotationInAnotherPackage, Annotation2InAnotherPackage +from tests.scoping.annotationCalls.acrossFiles.other import AnnotationInAnotherPackage, Annotation2InAnotherPackage, NotAnAnnotation // $TEST$ references safeds_MyAnnotation @»MyAnnotation« @@ -18,6 +18,9 @@ from tests.scoping.annotationCalls.acrossFiles.other import AnnotationInAnotherP // $TEST$ references other_Annotation2InAnotherPackage @»Annotation2InAnotherPackage« +// $TEST$ unresolved +@»NotAnAnnotation« + // $TEST$ unresolved @»AnnotationWithoutPackage« pipeline myPipeline {} diff --git a/tests/resources/scoping/annotation calls/across files/resource other package.sdstest b/tests/resources/scoping/annotation calls/across files/resource other package.sdstest index 50c473001..ca30cdcb3 100644 --- a/tests/resources/scoping/annotation calls/across files/resource other package.sdstest +++ b/tests/resources/scoping/annotation calls/across files/resource other package.sdstest @@ -8,3 +8,6 @@ annotation »AnnotationInAnotherPackage« // $TEST$ target other_Annotation2InAnotherPackage annotation »Annotation2InAnotherPackage« + +// $TEST$ target other_NotAnAnnotation +class »NotAnAnnotation« diff --git a/tests/resources/scoping/named types/across files/to global classes/main with qualified import.sdstest b/tests/resources/scoping/named types/across files/to global classes/main with qualified import.sdstest index 107722742..c29a526f2 100644 --- a/tests/resources/scoping/named types/across files/to global classes/main with qualified import.sdstest +++ b/tests/resources/scoping/named types/across files/to global classes/main with qualified import.sdstest @@ -1,7 +1,7 @@ package tests.scoping.namedTypes.acrossFiles.toGlobalClasses from safeds.scoping.namedTypes.acrossFiles.toGlobalClasses import MyClass -from tests.scoping.namedTypes.acrossFiles.toGlobalClasses.other import ClassInAnotherPackage, Class2InAnotherPackage +from tests.scoping.namedTypes.acrossFiles.toGlobalClasses.other import ClassInAnotherPackage, Class2InAnotherPackage, notANamedTypeDeclaration segment mySegment( // $TEST$ references safeds_MyClass @@ -20,5 +20,8 @@ segment mySegment( p5: »Class2InAnotherPackage«, // $TEST$ unresolved - p6: »ClassWithoutPackage«, + p6: »notANamedTypeDeclaration«, + + // $TEST$ unresolved + p7: »ClassWithoutPackage«, ) {} diff --git a/tests/resources/scoping/named types/across files/to global classes/resource other package.sdstest b/tests/resources/scoping/named types/across files/to global classes/resource other package.sdstest index 861695e02..06528eef9 100644 --- a/tests/resources/scoping/named types/across files/to global classes/resource other package.sdstest +++ b/tests/resources/scoping/named types/across files/to global classes/resource other package.sdstest @@ -8,3 +8,6 @@ class »ClassInAnotherPackage« // $TEST$ target other_Class2InAnotherPackage class »Class2InAnotherPackage« + +// $TEST$ target other_notANamedTypeDeclaration +fun »notANamedTypeDeclaration«() diff --git a/tests/resources/scoping/named types/across files/to global enums/main with qualified import.sdstest b/tests/resources/scoping/named types/across files/to global enums/main with qualified import.sdstest index 300f7286c..155e02e56 100644 --- a/tests/resources/scoping/named types/across files/to global enums/main with qualified import.sdstest +++ b/tests/resources/scoping/named types/across files/to global enums/main with qualified import.sdstest @@ -1,7 +1,7 @@ package tests.scoping.namedTypes.acrossFiles.toGlobalEnums from safeds.scoping.namedTypes.acrossFiles.toGlobalEnums import MyEnum -from tests.scoping.namedTypes.acrossFiles.toGlobalEnums.other import EnumInAnotherPackage, Enum2InAnotherPackage +from tests.scoping.namedTypes.acrossFiles.toGlobalEnums.other import EnumInAnotherPackage, Enum2InAnotherPackage, notANamedTypeDeclaration segment mySegment( // $TEST$ references safeds_MyEnum @@ -20,5 +20,8 @@ segment mySegment( p5: »Enum2InAnotherPackage«, // $TEST$ unresolved - p6: »EnumWithoutPackage«, + p6: »notANamedTypeDeclaration«, + + // $TEST$ unresolved + p7: »EnumWithoutPackage«, ) {} diff --git a/tests/resources/scoping/named types/across files/to global enums/resource other package.sdstest b/tests/resources/scoping/named types/across files/to global enums/resource other package.sdstest index 9d9d0c773..d5c036c00 100644 --- a/tests/resources/scoping/named types/across files/to global enums/resource other package.sdstest +++ b/tests/resources/scoping/named types/across files/to global enums/resource other package.sdstest @@ -8,3 +8,6 @@ enum »EnumInAnotherPackage« // $TEST$ target other_Enum2InAnotherPackage enum »Enum2InAnotherPackage« + +// $TEST$ target other_notANamedTypeDeclaration +fun »notANamedTypeDeclaration«()