Merge pull request #18767 from hvitved/ql4ql/inline-expectations

QL4QL: Add query suggesting use of inline test expectations

ql/ql/src/codeql_ql/ast/Yaml.qll

@@ -0,0 +1,67 @@
+/**
+ * Provides classes for working with YAML data.
+ *
+ * YAML documents are represented as abstract syntax trees whose nodes
+ * are either YAML values or alias nodes referring to another YAML value.
+ */
+
+private import codeql.yaml.Yaml as LibYaml
+
+private module YamlSig implements LibYaml::InputSig {
+  import codeql.Locations
+
+  class LocatableBase extends @yaml_locatable {
+    Location getLocation() { yaml_locations(this, result) }
+
+    string toString() { none() }
+  }
+
+  class NodeBase extends LocatableBase, @yaml_node {
+    NodeBase getChildNode(int i) { yaml(result, _, this, i, _, _) }
+
+    string getTag() { yaml(this, _, _, _, result, _) }
+
+    string getAnchor() { yaml_anchors(this, result) }
+
+    override string toString() { yaml(this, _, _, _, _, result) }
+  }
+
+  class ScalarNodeBase extends NodeBase, @yaml_scalar_node {
+    int getStyle() { yaml_scalars(this, result, _) }
+
+    string getValue() { yaml_scalars(this, _, result) }
+  }
+
+  class CollectionNodeBase extends NodeBase, @yaml_collection_node { }
+
+  class MappingNodeBase extends CollectionNodeBase, @yaml_mapping_node { }
+
+  class SequenceNodeBase extends CollectionNodeBase, @yaml_sequence_node { }
+
+  class AliasNodeBase extends NodeBase, @yaml_alias_node {
+    string getTarget() { yaml_aliases(this, result) }
+  }
+
+  class ParseErrorBase extends LocatableBase, @yaml_error {
+    string getMessage() { yaml_errors(this, result) }
+  }
+}
+
+import LibYaml::Make<YamlSig>
+
+/** A `.qlref` YAML document. */
+class QlRefDocument extends YamlDocument {
+  QlRefDocument() { this.getFile().getExtension() = "qlref" }
+
+  /** Holds if this `.qlref` file uses inline test expectations. */
+  predicate usesInlineExpectations() {
+    exists(YamlMapping n, YamlScalar value |
+      n.getDocument() = this and
+      value.getValue().matches("%InlineExpectations%")
+    |
+      value = n.lookup("postprocess")
+      or
+      value = n.lookup("postprocess").(YamlSequence).getElement(_)
+    )
+  }
+}

ql/ql/src/queries/style/QlRefInlineExpectations.ql

@@ -0,0 +1,15 @@
+/**
+ * @name Query test without inline test expectations
+ * @description Using inline test expectations is a best practice for writing query tests.
+ * @kind problem
+ * @problem.severity warning
+ * @id ql/qlref-inline-expectations
+ * @precision high
+ */
+
+import ql
+import codeql_ql.ast.Yaml
+
+from QlRefDocument f
+where not f.usesInlineExpectations()
+select f, "Query test does not use inline test expectations."

ql/ql/src/utils/test/InlineExpectationsTest.qll

@@ -3,33 +3,6 @@
  * See `shared/util/codeql/util/test/InlineExpectationsTest.qll`
  */
 
-private import ql as QL
 private import codeql.util.test.InlineExpectationsTest
-
-private module Impl implements InlineExpectationsTestSig {
-  private import codeql_ql.ast.internal.TreeSitter as TS
-
-  private newtype TExpectationComment = MkExpectationComment(TS::QL::LineComment comment)
-
-  /**
-   * Represents a line comment.
-   */
-  class ExpectationComment extends TExpectationComment {
-    TS::QL::LineComment comment;
-
-    ExpectationComment() { this = MkExpectationComment(comment) }
-
-    /** Returns the contents of the given comment, _without_ the preceding comment marker (`//`). */
-    string getContents() { result = comment.getValue().suffix(2) }
-
-    /** Gets a textual representation of this element. */
-    string toString() { result = comment.toString() }
-
-    /** Gets the location of this comment. */
-    Location getLocation() { result = comment.getLocation() }
-  }
-
-  class Location = QL::Location;
-}
-
+private import internal.InlineExpectationsTestImpl
 import Make<Impl>

ql/ql/src/utils/test/InlineExpectationsTestQuery.ql

@@ -0,0 +1,21 @@
+/**
+ * @kind test-postprocess
+ */
+
+private import ql
+private import codeql.util.test.InlineExpectationsTest as T
+private import internal.InlineExpectationsTestImpl
+import T::TestPostProcessing
+import T::TestPostProcessing::Make<Impl, Input>
+
+private module Input implements T::TestPostProcessing::InputSig<Impl> {
+  string getRelativeUrl(Location location) {
+    exists(File f, int startline, int startcolumn, int endline, int endcolumn |
+      location.hasLocationInfo(_, startline, startcolumn, endline, endcolumn) and
+      f = location.getFile()
+    |
+      result =
+        f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
+    )
+  }
+}

ql/ql/src/utils/test/internal/InlineExpectationsTestImpl.qll

@@ -0,0 +1,28 @@
+private import ql as QL
+private import codeql.util.test.InlineExpectationsTest
+
+module Impl implements InlineExpectationsTestSig {
+  private import codeql_ql.ast.internal.TreeSitter as TS
+
+  private newtype TExpectationComment = MkExpectationComment(TS::QL::LineComment comment)
+
+  /**
+   * Represents a line comment.
+   */
+  class ExpectationComment extends TExpectationComment {
+    TS::QL::LineComment comment;
+
+    ExpectationComment() { this = MkExpectationComment(comment) }
+
+    /** Returns the contents of the given comment, _without_ the preceding comment marker (`//`). */
+    string getContents() { result = comment.getValue().suffix(2) }
+
+    /** Gets a textual representation of this element. */
+    string toString() { result = comment.toString() }
+
+    /** Gets the location of this comment. */
+    Location getLocation() { result = comment.getLocation() }
+  }
+
+  class Location = QL::Location;
+}

ql/ql/test/queries/style/QlRefInlineExpectations/Foo.qll

@@ -0,0 +1 @@
+import ql

ql/ql/test/queries/style/QlRefInlineExpectations/QlRefInlineExpectations.expected

@@ -0,0 +1,2 @@
+| QlRefInlineExpectations.qlref:1:1:1:40 | queries ... ions.ql | Query test does not use inline test expectations. |
+| Test3.qlref:1:1:1:39 | query:  ... ists.ql | Query test does not use inline test expectations. |

ql/ql/test/queries/style/QlRefInlineExpectations/QlRefInlineExpectations.qlref

@@ -0,0 +1 @@
+queries/style/QlRefInlineExpectations.ql

ql/ql/test/queries/style/QlRefInlineExpectations/Test1.qlref

@@ -0,0 +1,2 @@
+query: queries/style/OmittableExists.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql

ql/ql/test/queries/style/QlRefInlineExpectations/Test2.qlref

@@ -0,0 +1,3 @@
+query: queries/style/OmittableExists.ql
+postprocess:
+  - utils/test/InlineExpectationsTestQuery.ql

ql/ql/test/queries/style/QlRefInlineExpectations/Test3.qlref

@@ -0,0 +1 @@
+query: queries/style/OmittableExists.ql

ql/tools/pre-finalize.cmd

@@ -2,6 +2,7 @@
 
 type NUL && "%CODEQL_DIST%\codeql" database index-files ^
     --include=**/qlpack.yml ^
+    --include-extension=.qlref ^
     --size-limit=5m ^
     --language yaml ^
     -- ^

ql/tools/pre-finalize.sh

@@ -4,6 +4,7 @@ set -eu
 
 "$CODEQL_DIST/codeql" database index-files \
     "--include=**/qlpack.yml" \
+    --include-extension=.qlref \
     --size-limit=5m \
     --language yaml \
     -- \

ql/tools/qltest.cmd

@@ -16,6 +16,7 @@ IF %ERRORLEVEL% NEQ 0 exit /b %ERRORLEVEL%
 type NUL && "%CODEQL_DIST%\codeql.exe" database index-files ^
     --prune=**/*.testproj ^
     --include-extension=.yml ^
+    --include-extension=.qlref ^
     --size-limit=5m ^
     --language=yaml ^
     --working-dir=. ^

ql/tools/qltest.sh

@@ -15,6 +15,7 @@ set -eu
 exec "${CODEQL_DIST}/codeql" database index-files \
     --prune="**/*.testproj" \
     --include-extension=.yml \
+    --include-extension=.qlref \
     --size-limit=5m \
     --language=yaml \
     --working-dir=.\