Skip to content

Commit

Permalink
fix: resolution of references to declarations of wrong node type (#599)
Browse files Browse the repository at this point in the history
### 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 <[email protected]>
  • Loading branch information
lars-reimann and megalinter-bot authored Oct 2, 2023
1 parent 4c61b8c commit 6ae387a
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 23 deletions.
39 changes: 21 additions & 18 deletions src/language/scoping/safe-ds-scope-provider.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
AstNode,
AstNodeDescription,
AstReflection,
DefaultScopeProvider,
EMPTY_SCOPE,
getContainerOfType,
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}

Expand All @@ -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,
});
}
}
Original file line number Diff line number Diff line change
@@ -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«
Expand All @@ -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 {}
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ annotation »AnnotationInAnotherPackage«

// $TEST$ target other_Annotation2InAnotherPackage
annotation »Annotation2InAnotherPackage«

// $TEST$ target other_NotAnAnnotation
class »NotAnAnnotation«
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -20,5 +20,8 @@ segment mySegment(
p5: »Class2InAnotherPackage«,

// $TEST$ unresolved
p6: »ClassWithoutPackage«,
p6: »notANamedTypeDeclaration«,

// $TEST$ unresolved
p7: »ClassWithoutPackage«,
) {}
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ class »ClassInAnotherPackage«

// $TEST$ target other_Class2InAnotherPackage
class »Class2InAnotherPackage«

// $TEST$ target other_notANamedTypeDeclaration
fun »notANamedTypeDeclaration«()
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -20,5 +20,8 @@ segment mySegment(
p5: »Enum2InAnotherPackage«,

// $TEST$ unresolved
p6: »EnumWithoutPackage«,
p6: »notANamedTypeDeclaration«,

// $TEST$ unresolved
p7: »EnumWithoutPackage«,
) {}
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ enum »EnumInAnotherPackage«

// $TEST$ target other_Enum2InAnotherPackage
enum »Enum2InAnotherPackage«

// $TEST$ target other_notANamedTypeDeclaration
fun »notANamedTypeDeclaration«()

0 comments on commit 6ae387a

Please sign in to comment.