diff --git a/unified/ql/lib/codeql/unified/internal/StaticNameBinding.qll b/unified/ql/lib/codeql/unified/internal/StaticNameBinding.qll index 907cc9f14010..ca9cca2fc61a 100644 --- a/unified/ql/lib/codeql/unified/internal/StaticNameBinding.qll +++ b/unified/ql/lib/codeql/unified/internal/StaticNameBinding.qll @@ -16,6 +16,7 @@ private newtype TNameBindingNode = n instanceof ClassLikeDeclaration } or TModuleScope(ModuleScopeRepr repr) or + TFolderScope(Folder folder) or TModuleRoot() /** @@ -39,6 +40,9 @@ class NameBindingNode extends TNameBindingNode { /** Holds if this represents the given module scope. */ predicate isModuleScopeNode(ModuleScopeRepr repr) { this = TModuleScope(repr) } + /** Holds if this represents the set of members that can be accessed unqualified within the given folder and subfolders. */ + predicate isFolderScope(Folder folder) { this = TFolderScope(folder) } + /** Holds if this represents the root namespace in which all named modules are members. */ predicate isModuleRoot() { this = TModuleRoot() } @@ -76,6 +80,8 @@ class NameBindingNode extends TNameBindingNode { this.isModuleScopeNode(repr) and result = "ModuleScope(" + repr + ")" ) or + exists(Folder folder | this.isFolderScope(folder) and result = "FolderScope(" + folder + ")") + or this.isModuleRoot() and result = "ModuleRoot" } @@ -173,6 +179,8 @@ predicate storeStep(NameBindingNode node1, string name, NameBindingNode node2) { mod.hasImportableName(name) and node2.isModuleRoot() ) + or + FolderHeuristic::storeStep(node1, name, node2) } predicate valueStep(NameBindingNode node1, NameBindingNode node2) { @@ -224,6 +232,8 @@ predicate valueStep(NameBindingNode node1, NameBindingNode node2) { node1 = getNodeFromRef(p) and node2 = getNodeFromRef(p.getSubPattern()) ) + or + FolderHeuristic::valueStep(node1, node2) } private predicate isImportPrefix(Expr e) { @@ -403,3 +413,96 @@ module DebugGraph { ) } } + +/** + * Implements a folder-based heuristic to linking up top-level names + * between files that are not included in any module scope. + */ +private module FolderHeuristic { + private predicate topLevelNameDef(File file, string name, NameBindingNode node) { + exists(TopLevel top, Stmt stmt, NameDeclaration nameDecl | + top.getFile() = file and + stmt = top.getBody().getAStmt() and + not stmt.(ClassLikeDeclaration).hasModifier("extension") and // TODO: target of type extensions should not be seen as a NameDeclaration + not isPrivateToLocalScope(nameDecl) and + nameDecl.getDeclaration() = stmt and + name = nameDecl.getName() and + node.isIdentifier(nameDecl) + ) + } + + private predicate uniqueTopLevelName(File file, string name) { + file = unique(File f | topLevelNameDef(f, name, _)) + } + + /** + * Holds if `file` has a one of the definitions of the given ambiguous name. + * + * A name is considered "ambiguous" if there is more than one file exporting it. + */ + private predicate ambiguousTopLevelName(File file, string name) { + topLevelNameDef(file, name, _) and + not uniqueTopLevelName(file, name) + } + + /** Holds if `folder` contains one or more definitions of the given ambiguous name */ + private predicate containsDef(Folder folder, string name) { + exists(File f | + ambiguousTopLevelName(f, name) and + folder = f.getParentContainer+() + ) + } + + /** + * Holds if `folder` has two or more subfolders containing a definition of `name`. + */ + private predicate hasConflictingDefs(Folder folder, string name) { + containsDef(folder, name) and + not exists(unique(Folder child | child = folder.getAFolder() and containsDef(child, name))) + } + + /** + * Holds if `folder` is an outermost folder containing exactly one definition of `name`. + * + * This means `folder` should act as the scope of that definition. + */ + private predicate isOutermostNonConflictingScope(Folder folder, string name) { + containsDef(folder, name) and + hasConflictingDefs(folder.getParentContainer(), name) and + not hasConflictingDefs(folder, name) + } + + /** + * Gets the scope into which a definition of `name` appearing in `folder` should target. + */ + private Folder getOutermostNonConflictingScope(Folder folder, string name) { + isOutermostNonConflictingScope(folder, name) and + result = folder + or + result = getOutermostNonConflictingScope(folder.getParentContainer(), name) and + not isOutermostNonConflictingScope(folder, name) and + containsDef(folder, name) // Prune to the subfolder actually containing the definition + } + + predicate storeStep(NameBindingNode node1, string name, NameBindingNode node2) { + exists(File file | topLevelNameDef(file, name, node1) | + node2.isFolderScope(getOutermostNonConflictingScope(file.getParentContainer(), name)) + or + uniqueTopLevelName(file, name) and + node2.isFolderScope(any(Folder f | f.getRelativePath() = "")) + ) + } + + predicate valueStep(NameBindingNode node1, NameBindingNode node2) { + exists(TopLevel top | + node1.isFolderScope(top.getFile().getParentContainer()) and + node2.isLocalNamespace(top.getBody()) and + not top.getFile() = any(ModuleScopeRepr r).getAnIncludedFile() + ) + or + exists(Folder folder | + node1.isFolderScope(folder.getParentContainer()) and + node2.isFolderScope(folder) + ) + } +} diff --git a/unified/ql/src/meta/FilesInModuleScope.ql b/unified/ql/src/meta/FilesInModuleScope.ql new file mode 100644 index 000000000000..8b050820d2c0 --- /dev/null +++ b/unified/ql/src/meta/FilesInModuleScope.ql @@ -0,0 +1,23 @@ +/** + * @name Files in module scope + * @description Files that are part of a module scope + * @kind problem + * @problem.severity recommendation + * @id unified/meta/files-in-module-scope + * @tags meta + * @precision very-low + */ + +private import unified +private import codeql.unified.internal.NameBindingPlugin + +string getModuleName(ModuleScopeRepr scope) { + result = strictconcat(string n | scope.hasImportableName(n) | n, ",") + or + not scope.hasImportableName(_) and + result = scope.getFile().getRelativePath() + ":" + scope.getLocation().getStartColumn() +} + +from File file, ModuleScopeRepr scope +where scope.getAnIncludedFile() = file +select file, "Included in $@", scope, getModuleName(scope) diff --git a/unified/ql/src/meta/FilesNotInModuleScope.ql b/unified/ql/src/meta/FilesNotInModuleScope.ql new file mode 100644 index 000000000000..d46765cf3d3a --- /dev/null +++ b/unified/ql/src/meta/FilesNotInModuleScope.ql @@ -0,0 +1,18 @@ +/** + * @name Files not in any module scope + * @description Files that are not part of any module scope + * @kind problem + * @problem.severity recommendation + * @id unified/meta/files-not-in-module-scope + * @tags meta + * @precision very-low + */ + +private import unified +private import codeql.unified.internal.NameBindingPlugin + +from File file +where + not file = any(ModuleScopeRepr s).getAnIncludedFile() and + file = any(TopLevel t).getFile() +select file, "Not included in any module scope" diff --git a/unified/ql/src/meta/ImportUtil.qll b/unified/ql/src/meta/ImportUtil.qll new file mode 100644 index 000000000000..e472a812e140 --- /dev/null +++ b/unified/ql/src/meta/ImportUtil.qll @@ -0,0 +1,16 @@ +private import unified +private import codeql.unified.internal.StaticNameBinding +private import codeql.unified.internal.NameBindingPlugin + +private Expr getAnImportedPrefix(ImportDeclaration imprt) { + result = imprt.getImportedExpr() + or + result = getAnImportedPrefix(imprt).(MemberAccessExpr).getBase() +} + +ModuleScopeRepr getImportedModule(ImportDeclaration imprt) { + exists(NamespaceNode node | + node.isModuleScopeNode(result) and + node.ref().asIdentifier() = getAnImportedPrefix(imprt).(NameExpr).getIdentifier() + ) +} diff --git a/unified/ql/src/meta/ImportsAmbiguous.ql b/unified/ql/src/meta/ImportsAmbiguous.ql new file mode 100644 index 000000000000..f5ec097fab21 --- /dev/null +++ b/unified/ql/src/meta/ImportsAmbiguous.ql @@ -0,0 +1,16 @@ +/** + * @name Ambiguous imports + * @description Imports for which multiple target modules were found + * @kind problem + * @problem.severity recommendation + * @id unified/meta/ambiguous-imports + * @tags meta + * @precision very-low + */ + +private import unified +private import ImportUtil + +from ImportDeclaration imprt +where strictcount(getImportedModule(imprt)) > 1 +select imprt, "Unresolved import" diff --git a/unified/ql/src/meta/ImportsResolved.ql b/unified/ql/src/meta/ImportsResolved.ql new file mode 100644 index 000000000000..fd9bb22e6627 --- /dev/null +++ b/unified/ql/src/meta/ImportsResolved.ql @@ -0,0 +1,16 @@ +/** + * @name Resolved imports + * @description Imports whose target module was found + * @kind problem + * @problem.severity recommendation + * @id unified/meta/resolved-imports + * @tags meta + * @precision very-low + */ + +private import unified +private import ImportUtil + +from ImportDeclaration imprt +where exists(getImportedModule(imprt)) +select imprt, "Resolved import" diff --git a/unified/ql/src/meta/ImportsUnresolved.ql b/unified/ql/src/meta/ImportsUnresolved.ql new file mode 100644 index 000000000000..8295d078b533 --- /dev/null +++ b/unified/ql/src/meta/ImportsUnresolved.ql @@ -0,0 +1,16 @@ +/** + * @name Unresolved imports + * @description Imports whose target module could not be found + * @kind problem + * @problem.severity recommendation + * @id unified/meta/unresolved-imports + * @tags meta + * @precision very-low + */ + +private import unified +private import ImportUtil + +from ImportDeclaration imprt +where not exists(getImportedModule(imprt)) +select imprt, "Unresolved import" diff --git a/unified/ql/src/meta/StaticNamesResolved.ql b/unified/ql/src/meta/StaticNamesResolved.ql new file mode 100644 index 000000000000..5aa9093f1a71 --- /dev/null +++ b/unified/ql/src/meta/StaticNamesResolved.ql @@ -0,0 +1,32 @@ +/** + * @name Resolved static name references + * @description Static name references that could be resolved to a target + * @kind problem + * @problem.severity recommendation + * @id unified/meta/static-names-resolved + * @tags meta + * @precision very-low + */ + +private import unified +private import codeql.unified.internal.StaticNameBinding +private import codeql.unified.internal.NameBindingPlugin + +AstNode getResolutionTarget(Identifier id) { + exists(NameBindingNode node | node.isIdentifier(id) | + trackNameDeclaration(result) = node and + exists(ClassLikeDeclaration cls | + not cls.hasModifier("extension") and // don't treat extensions as the true resolution target + result = cls.getName() + ) + or + exists(NamespaceNode ns | + ns.isModuleScopeNode(result) and + ns.ref() = node + ) + ) +} + +from Identifier id, AstNode target +where target = getResolutionTarget(id) +select id, "Reference to $@", target, target.toString() diff --git a/unified/ql/test/library-tests/static-name-binding/not-a-package/Main/Drivers/Driver.swift b/unified/ql/test/library-tests/static-name-binding/not-a-package/Main/Drivers/Driver.swift new file mode 100644 index 000000000000..4b41a6324afb --- /dev/null +++ b/unified/ql/test/library-tests/static-name-binding/not-a-package/Main/Drivers/Driver.swift @@ -0,0 +1,5 @@ +class Driver { // name=Main.Driver + class Nested {} // name=Main.Driver.Nested +} + +class UniqueToMain {} diff --git a/unified/ql/test/library-tests/static-name-binding/not-a-package/Main/Runner.swift b/unified/ql/test/library-tests/static-name-binding/not-a-package/Main/Runner.swift new file mode 100644 index 000000000000..894e6257dbb1 --- /dev/null +++ b/unified/ql/test/library-tests/static-name-binding/not-a-package/Main/Runner.swift @@ -0,0 +1,10 @@ +func main() { + Driver(); // $ access=Main.Driver + Driver.Nested(); // $ access=Main.Driver access=Main.Driver.Nested + UniqueToMain(); // $ access=UniqueToMain + UniqueToMock(); // $ access=UniqueToMock +} + +class MyDriver: Driver { // $ access=Main.Driver + class B: Nested {} // $ access=Main.Driver.Nested +} diff --git a/unified/ql/test/library-tests/static-name-binding/not-a-package/Main/Util/Util.swift b/unified/ql/test/library-tests/static-name-binding/not-a-package/Main/Util/Util.swift new file mode 100644 index 000000000000..adc7a3004127 --- /dev/null +++ b/unified/ql/test/library-tests/static-name-binding/not-a-package/Main/Util/Util.swift @@ -0,0 +1,3 @@ +func getDriver() -> Driver { // $ access=Main.Driver + return Driver() // $ access=Main.Driver +} diff --git a/unified/ql/test/library-tests/static-name-binding/not-a-package/Mock/Drivers/Driver.swift b/unified/ql/test/library-tests/static-name-binding/not-a-package/Mock/Drivers/Driver.swift new file mode 100644 index 000000000000..75d9e81affe5 --- /dev/null +++ b/unified/ql/test/library-tests/static-name-binding/not-a-package/Mock/Drivers/Driver.swift @@ -0,0 +1,5 @@ +class Driver { // name=Mock.Driver + class Nested {} // name=Mock.Driver.Nested +} + +class UniqueToMock {} diff --git a/unified/ql/test/library-tests/static-name-binding/not-a-package/Mock/Runner.swift b/unified/ql/test/library-tests/static-name-binding/not-a-package/Mock/Runner.swift new file mode 100644 index 000000000000..930301ab9015 --- /dev/null +++ b/unified/ql/test/library-tests/static-name-binding/not-a-package/Mock/Runner.swift @@ -0,0 +1,10 @@ +func main() { + Driver(); // $ access=Mock.Driver + Driver.Nested(); // $ access=Mock.Driver access=Mock.Driver.Nested + UniqueToMain(); // $ access=UniqueToMain + UniqueToMock(); // $ access=UniqueToMock +} + +class MyDriver: Driver { // $ access=Mock.Driver + class B: Nested {} // $ access=Mock.Driver.Nested +} diff --git a/unified/ql/test/library-tests/static-name-binding/not-a-package/Mock/Util/Util.swift b/unified/ql/test/library-tests/static-name-binding/not-a-package/Mock/Util/Util.swift new file mode 100644 index 000000000000..1d83c82c4f9d --- /dev/null +++ b/unified/ql/test/library-tests/static-name-binding/not-a-package/Mock/Util/Util.swift @@ -0,0 +1,3 @@ +func getDriver() -> Driver { // $ access=Mock.Driver + return Driver() // $ access=Mock.Driver +} diff --git a/unified/ql/test/library-tests/static-name-binding/not-a-package/SiblingFiles/Def1.swift b/unified/ql/test/library-tests/static-name-binding/not-a-package/SiblingFiles/Def1.swift new file mode 100644 index 000000000000..6edb39c550d7 --- /dev/null +++ b/unified/ql/test/library-tests/static-name-binding/not-a-package/SiblingFiles/Def1.swift @@ -0,0 +1,3 @@ +class DeclaredTwiceInSameFolder {} // name=Def1.DeclaredTwiceInSameFolder + +class OnlyInDef1 {} diff --git a/unified/ql/test/library-tests/static-name-binding/not-a-package/SiblingFiles/Def2.swift b/unified/ql/test/library-tests/static-name-binding/not-a-package/SiblingFiles/Def2.swift new file mode 100644 index 000000000000..f41e33bc34bb --- /dev/null +++ b/unified/ql/test/library-tests/static-name-binding/not-a-package/SiblingFiles/Def2.swift @@ -0,0 +1,3 @@ +class DeclaredTwiceInSameFolder {} // name=Def2.DeclaredTwiceInSameFolder + +class OnlyInDef2 {} diff --git a/unified/ql/test/library-tests/static-name-binding/not-a-package/SiblingFiles/SubFolder1/Def.swift b/unified/ql/test/library-tests/static-name-binding/not-a-package/SiblingFiles/SubFolder1/Def.swift new file mode 100644 index 000000000000..139c0d2ab378 --- /dev/null +++ b/unified/ql/test/library-tests/static-name-binding/not-a-package/SiblingFiles/SubFolder1/Def.swift @@ -0,0 +1,3 @@ +class DeclaredTwiceInSubFolder {} // name=Subfolder2.DeclaredTwiceInSubFolder + +class OnlyInSubFolder2 {} diff --git a/unified/ql/test/library-tests/static-name-binding/not-a-package/SiblingFiles/SubFolder2/Def.swift b/unified/ql/test/library-tests/static-name-binding/not-a-package/SiblingFiles/SubFolder2/Def.swift new file mode 100644 index 000000000000..07e2510e5cd1 --- /dev/null +++ b/unified/ql/test/library-tests/static-name-binding/not-a-package/SiblingFiles/SubFolder2/Def.swift @@ -0,0 +1,3 @@ +class DeclaredTwiceInSubFolder {} // name=Subfolder1.DeclaredTwiceInSubFolder + +class OnlyInSubFolder1 {} diff --git a/unified/ql/test/library-tests/static-name-binding/not-a-package/SiblingFiles/Use.swift b/unified/ql/test/library-tests/static-name-binding/not-a-package/SiblingFiles/Use.swift new file mode 100644 index 000000000000..897386e9e93e --- /dev/null +++ b/unified/ql/test/library-tests/static-name-binding/not-a-package/SiblingFiles/Use.swift @@ -0,0 +1,9 @@ +private protocol P { + let x1: DeclaredTwiceInSameFolder; // unresolved; ambiguous reference + let x2: OnlyInDef1; // $ access=OnlyInDef1 + let x3: OnlyInDef2; // $ access=OnlyInDef2 + + let x4: DeclaredTwiceInSubFolder; // unresolved; ambiguous reference + let x5: OnlyInSubFolder1; // $ access=OnlyInSubFolder1 + let x6: OnlyInSubFolder2; // $ access=OnlyInSubFolder2 +} diff --git a/unified/ql/test/library-tests/static-name-binding/unqualified-access.swift b/unified/ql/test/library-tests/static-name-binding/unqualified-access.swift index 732e65563b3b..86cadc0b7a1f 100644 --- a/unified/ql/test/library-tests/static-name-binding/unqualified-access.swift +++ b/unified/ql/test/library-tests/static-name-binding/unqualified-access.swift @@ -10,7 +10,7 @@ class ASub : A { // $ access=A class BSub : B { // $ access=A.B let x3: B = nil; // $ access=A.B - let x4: C = nil; // $ access=A.B.C + let x4: C = nil; // $ access=A.B.C SPURIOUS: access=Target3.C // spurious result from folder-based heuristic } class BSub2 : B { // $ access=A.B