Merge pull request #15043 from hvitved/ql/redundant-import

hvitved · web-flow · commit 7819dcf0a77b · 2023-12-09T12:15:09.000+01:00
QL4QL: Improvements to `RedundantImport` query
diff --git a/javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/EndpointCharacteristics.qll b/javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/EndpointCharacteristics.qll
@@ -6,14 +6,12 @@ import experimental.adaptivethreatmodeling.EndpointTypes
 private import semmle.javascript.security.dataflow.SqlInjectionCustomizations
 private import semmle.javascript.security.dataflow.DomBasedXssCustomizations
 private import semmle.javascript.security.dataflow.NosqlInjectionCustomizations
-private import semmle.javascript.security.dataflow.TaintedPathCustomizations
 private import semmle.javascript.heuristics.SyntacticHeuristics as SyntacticHeuristics
 private import semmle.javascript.filters.ClassifyFiles as ClassifyFiles
 private import semmle.javascript.security.dataflow.XxeCustomizations
 private import semmle.javascript.security.dataflow.RemotePropertyInjectionCustomizations
 private import semmle.javascript.security.dataflow.TypeConfusionThroughParameterTamperingCustomizations
 private import semmle.javascript.security.dataflow.ZipSlipCustomizations
-private import semmle.javascript.security.dataflow.TaintedPathCustomizations
 private import semmle.javascript.security.dataflow.CleartextLoggingCustomizations
 private import semmle.javascript.security.dataflow.XpathInjectionCustomizations
 private import semmle.javascript.security.dataflow.Xss::Shared as Xss
@@ -28,10 +26,8 @@ private import semmle.javascript.security.dataflow.CommandInjectionCustomization
 private import semmle.javascript.security.dataflow.PrototypePollutionCustomizations
 private import semmle.javascript.security.dataflow.UnvalidatedDynamicMethodCallCustomizations
 private import semmle.javascript.security.dataflow.TaintedFormatStringCustomizations
-private import semmle.javascript.security.dataflow.NosqlInjectionCustomizations
 private import semmle.javascript.security.dataflow.PostMessageStarCustomizations
 private import semmle.javascript.security.dataflow.RegExpInjectionCustomizations
-private import semmle.javascript.security.dataflow.SqlInjectionCustomizations
 private import semmle.javascript.security.dataflow.InsecureRandomnessCustomizations
 private import semmle.javascript.security.dataflow.XmlBombCustomizations
 private import semmle.javascript.security.dataflow.InsufficientPasswordHashCustomizations
diff --git a/javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/ExtractMisclassifiedEndpointFeatures.ql b/javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/ExtractMisclassifiedEndpointFeatures.ql
@@ -6,10 +6,8 @@
 
 import javascript
 import experimental.adaptivethreatmodeling.AdaptiveThreatModeling
-import experimental.adaptivethreatmodeling.ATMConfig
 import experimental.adaptivethreatmodeling.BaseScoring
 import experimental.adaptivethreatmodeling.EndpointFeatures as EndpointFeatures
-import experimental.adaptivethreatmodeling.EndpointTypes
 import semmle.javascript.security.dataflow.NosqlInjectionCustomizations
 
 /** Gets the positive endpoint type for which you wish to find misclassified examples. */
diff --git a/javascript/ql/experimental/adaptivethreatmodeling/test/endpoint_large_scale/FilteredTruePositives.ql b/javascript/ql/experimental/adaptivethreatmodeling/test/endpoint_large_scale/FilteredTruePositives.ql
@@ -14,7 +14,6 @@
 
 import semmle.javascript.security.dataflow.NosqlInjectionCustomizations
 import semmle.javascript.security.dataflow.SqlInjectionCustomizations
-import semmle.javascript.security.dataflow.TaintedPathCustomizations
 import semmle.javascript.security.dataflow.DomBasedXssCustomizations
 import semmle.javascript.security.dataflow.ShellCommandInjectionFromEnvironmentCustomizations
 import experimental.adaptivethreatmodeling.NosqlInjectionATM as NosqlInjectionAtm
diff --git a/javascript/ql/lib/semmle/javascript/dataflow/Configuration.qll b/javascript/ql/lib/semmle/javascript/dataflow/Configuration.qll
@@ -71,7 +71,6 @@
 private import javascript
 private import internal.FlowSteps
 private import internal.AccessPaths
-private import internal.CallGraphs
 private import semmle.javascript.Unit
 private import semmle.javascript.internal.CachedStages
 
diff --git a/javascript/ql/lib/semmle/javascript/dataflow/TypeInference.qll b/javascript/ql/lib/semmle/javascript/dataflow/TypeInference.qll
@@ -27,10 +27,8 @@
 
 private import javascript
 import AbstractValues
-import AbstractProperties
 private import InferredTypes
 private import Refinements
-private import internal.AbstractValuesImpl
 import internal.BasicExprTypeInference
 import internal.InterModuleTypeInference
 import internal.InterProceduralTypeInference
diff --git a/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/DependencyInjections.qll b/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/DependencyInjections.qll
@@ -9,7 +9,6 @@
 
 import javascript
 private import AngularJS
-private import ServiceDefinitions
 
 /**
  * Holds if `nd` is an `angular.injector()` value
diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Express.qll b/javascript/ql/lib/semmle/javascript/frameworks/Express.qll
@@ -3,7 +3,6 @@
  */
 
 import javascript
-import semmle.javascript.frameworks.HTTP
 import semmle.javascript.frameworks.ExpressModules
 private import semmle.javascript.dataflow.InferredTypes
 private import semmle.javascript.frameworks.ConnectExpressShared::ConnectExpressShared
diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/LoopBoundInjectionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/LoopBoundInjectionQuery.qll
@@ -8,7 +8,6 @@
  */
 
 import javascript
-import semmle.javascript.security.TaintedObject
 import LoopBoundInjectionCustomizations::LoopBoundInjection
 
 /**
diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionCustomizations.qll
@@ -6,7 +6,6 @@
 
 import javascript
 import semmle.javascript.security.TaintedObject
-import semmle.javascript.dependencies.Dependencies
 import semmle.javascript.dependencies.SemVer
 
 module PrototypePollution {
diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionQuery.qll
@@ -9,7 +9,6 @@
 
 import javascript
 import semmle.javascript.security.TaintedObject
-import semmle.javascript.dependencies.Dependencies
 import semmle.javascript.dependencies.SemVer
 import PrototypePollutionCustomizations::PrototypePollution
 
diff --git a/javascript/ql/src/meta/analysis-quality/UnmodelledSteps.ql b/javascript/ql/src/meta/analysis-quality/UnmodelledSteps.ql
@@ -9,7 +9,6 @@
  */
 
 import javascript
-import meta.MetaMetrics
 private import Expressions.ExprHasNoEffect
 import meta.internal.TaintMetrics
 
diff --git a/javascript/ql/test/library-tests/DependencyModuleImports/Test.ql b/javascript/ql/test/library-tests/DependencyModuleImports/Test.ql
@@ -1,5 +1,4 @@
 import javascript
-import semmle.javascript.dependencies.Dependencies
 import semmle.javascript.dependencies.SemVer
 
 class SampleVersionSink extends DataFlow::Node {
diff --git a/ql/ql/src/codeql_ql/ast/Ast.qll b/ql/ql/src/codeql_ql/ast/Ast.qll
@@ -889,6 +889,9 @@ class ModuleMember extends TModuleMember, AstNode {
 
   /** Holds if this member is declared as `final`. */
   predicate isFinal() { this.hasAnnotation("final") }
+
+  /** Holds if this member is declared as `deprecated`. */
+  predicate isDeprecated() { this.hasAnnotation("deprecated") }
 }
 
 private newtype TDeclarationKind =
@@ -2738,6 +2741,18 @@ module YAML {
       )
     }
 
+    /**
+     * Gets the language library file for this QLPack, if any. For example, the
+     * language library file for `codeql/cpp-all` is `cpp.qll`.
+     */
+    File getLanguageLib() {
+      exists(string name |
+        name = this.getExtractor() and
+        result.getParentContainer() = this.getFile().getParentContainer() and
+        result.getBaseName() = name + ".qll"
+      )
+    }
+
     Location getLocation() {
       // hacky, just pick the first node in the file.
       result =
diff --git a/ql/ql/src/codeql_ql/style/RedundantImportQuery.qll b/ql/ql/src/codeql_ql/style/RedundantImportQuery.qll
@@ -1,68 +1,64 @@
 import ql
+private import YAML
+private import codeql_ql.ast.internal.Module
 
-Import imports(Import imp) {
+private FileOrModule getResolvedModule(Import imp) {
+  result = imp.getResolvedModule() and
+  // skip the top-level language files
+  not result.asFile() = any(QLPack p).getLanguageLib()
+}
+
+private Import imports(Import imp) {
   (
     exists(File file, TopLevel top |
-      imp.getResolvedModule().asFile() = file and
+      getResolvedModule(imp).asFile() = file and
       top.getLocation().getFile() = file and
       result = top.getAMember()
     )
     or
     exists(Module mod |
-      imp.getResolvedModule().asModule() = mod and
+      getResolvedModule(imp).asModule() = mod and
       result = mod.getAMember()
     )
   )
 }
 
-Import getAnImport(AstNode parent) {
+private Import getAnImport(AstNode parent) {
   result = parent.(TopLevel).getAMember()
   or
   result = parent.(Module).getAMember()
 }
 
-pragma[inline]
-predicate importsFromSameFolder(Import a, Import b) {
-  exists(string base |
-    a.getImportString().regexpCapture("(.*)\\.[^\\.]*", 1) = base and
-    b.getImportString().regexpCapture("(.*)\\.[^\\.]*", 1) = base
-  )
-  or
-  not a.getImportString().matches("%.%") and
-  not b.getImportString().matches("%.%")
-}
-
 predicate problem(Import imp, Import redundant, string message) {
   not exists(imp.importedAs()) and
   not exists(redundant.importedAs()) and
   not exists(imp.getModuleExpr().getQualifier*().getArgument(_)) and // any type-arguments, and we ignore, they might be different.
   // skip the top-level language files, they have redundant imports, and that's fine.
-  not exists(imp.getLocation().getFile().getParentContainer().getFile("qlpack.yml")) and
+  not imp.getLocation().getFile() = any(QLPack p).getLanguageLib() and
   // skip the DataFlowImpl.qll and similar, they have redundant imports in some copies.
   not imp.getLocation()
       .getFile()
       .getBaseName()
       .regexpMatch([".*Impl\\d?\\.qll", "DataFlowImpl.*\\.qll"]) and
-  // skip two imports that imports different things from the same folder.
-  not importsFromSameFolder(imp, redundant) and
   // if the redundant is public, and the imp is private, then the redundant might add things that are exported.
   not (imp.isPrivate() and not redundant.isPrivate()) and
   // Actually checking if the import is redundant:
   exists(AstNode parent |
     imp = getAnImport(parent) and
-    redundant = getAnImport(parent) and
-    redundant.getLocation().getStartLine() > imp.getLocation().getStartLine()
+    redundant = getAnImport(parent)
   |
     message = "Redundant import, the module is already imported inside $@." and
     // only looking for things directly imported one level down. Otherwise things gets complicated (lots of cycles).
     exists(Import inner | inner = imports(imp) |
-      redundant.getResolvedModule() = inner.getResolvedModule() and
+      getResolvedModule(redundant) = getResolvedModule(inner) and
       not inner.isPrivate() and // if the inner is private, then it's not propagated out.
+      not inner.isDeprecated() and
       not exists(inner.importedAs())
     )
     or
     message = "Duplicate import, the module is already imported by $@." and
     // two different import statements, that import the same thing
-    imp.getResolvedModule() = redundant.getResolvedModule()
+    getResolvedModule(imp) = getResolvedModule(redundant) and
+    redundant.getLocation().getStartLine() > imp.getLocation().getStartLine()
   )
 }
diff --git a/ql/ql/test/queries/style/RedundantImport/D.qll b/ql/ql/test/queries/style/RedundantImport/D.qll
@@ -0,0 +1,2 @@
+import folder.A
+import folder.B
diff --git a/ql/ql/test/queries/style/RedundantImport/E.qll b/ql/ql/test/queries/style/RedundantImport/E.qll
@@ -0,0 +1,2 @@
+import folder.A
+import folder.C
diff --git a/ql/ql/test/queries/style/RedundantImport/RedundantImport.expected b/ql/ql/test/queries/style/RedundantImport/RedundantImport.expected
@@ -0,0 +1 @@
+| D.qll:1:1:1:15 | Import | Redundant import, the module is already imported inside $@. | D.qll:2:1:2:15 | Import | folder.B |
diff --git a/ql/ql/test/queries/style/RedundantImport/RedundantImport.qlref b/ql/ql/test/queries/style/RedundantImport/RedundantImport.qlref
@@ -0,0 +1 @@
+queries/style/RedundantImport.ql
diff --git a/ql/ql/test/queries/style/RedundantImport/folder/A.qll b/ql/ql/test/queries/style/RedundantImport/folder/A.qll
@@ -0,0 +1 @@
+predicate p() { any() }
diff --git a/ql/ql/test/queries/style/RedundantImport/folder/B.qll b/ql/ql/test/queries/style/RedundantImport/folder/B.qll
@@ -0,0 +1 @@
+import A
diff --git a/ql/ql/test/queries/style/RedundantImport/folder/C.qll b/ql/ql/test/queries/style/RedundantImport/folder/C.qll
@@ -0,0 +1 @@
+deprecated import A
