From 367027e777aa5aa934c5c1c5e7924031571d6f3a Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Mon, 1 Sep 2025 15:10:10 +0200 Subject: [PATCH] Ql: Fix some Ql4Ql violations. --- .../codeql_ql/style/RedundantCastQuery.qll | 24 +++++++++---------- .../performance/VarUnusedInDisjunct/Test.qll | 2 ++ .../style/Misspelling/Misspelling.expected | 4 ++-- ql/ql/test/queries/style/Misspelling/Test.qll | 2 ++ .../style/UseInstanceofExtension/Foo.qll | 2 ++ .../UseInstanceofExtension.expected | 2 +- .../UseSetLiteral/UseSetLiteral.expected | 4 ++-- .../test/queries/style/UseSetLiteral/test.qll | 2 ++ 8 files changed, 25 insertions(+), 17 deletions(-) diff --git a/ql/ql/src/codeql_ql/style/RedundantCastQuery.qll b/ql/ql/src/codeql_ql/style/RedundantCastQuery.qll index 9549bb85844b..4805cf5d69f5 100644 --- a/ql/ql/src/codeql_ql/style/RedundantCastQuery.qll +++ b/ql/ql/src/codeql_ql/style/RedundantCastQuery.qll @@ -1,11 +1,12 @@ import ql class RedundantInlineCast extends AstNode instanceof InlineCast { - Type t; - RedundantInlineCast() { - t = unique( | | super.getType()) and - ( + exists(Type t | + t = unique( | | super.getType()) and + // noopt can require explicit casts + not this.getEnclosingPredicate().getAnAnnotation() instanceof NoOpt + | // The cast is to the type the base expression already has t = unique( | | super.getBase().getType()) or @@ -23,9 +24,7 @@ class RedundantInlineCast extends AstNode instanceof InlineCast { target = unique( | | call.getTarget()) and t = unique( | | target.getParameterType(i)) ) - ) and - // noopt can require explicit casts - not this.getEnclosingPredicate().getAnAnnotation() instanceof NoOpt + ) } TypeExpr getTypeExpr() { result = super.getTypeExpr() } @@ -49,15 +48,16 @@ private class AnyCast extends AstNode instanceof FullAggregate { // `foo = any(Bar b)` is effectively a cast to `Bar`. class RedundantAnyCast extends AstNode instanceof ComparisonFormula { AnyCast cast; - Expr operand; RedundantAnyCast() { super.getOperator() = "=" and super.getAnOperand() = cast and - super.getAnOperand() = operand and - cast != operand and - unique( | | operand.getType()).getASuperType*() = - unique( | | cast.getTypeExpr().getResolvedType()) and + exists(Expr operand | + super.getAnOperand() = operand and + cast != operand and + unique( | | operand.getType()).getASuperType*() = + unique( | | cast.getTypeExpr().getResolvedType()) + ) and not this.getEnclosingPredicate().getAnAnnotation() instanceof NoOpt } diff --git a/ql/ql/test/queries/performance/VarUnusedInDisjunct/Test.qll b/ql/ql/test/queries/performance/VarUnusedInDisjunct/Test.qll index 520acbaa1b3a..10e97e582096 100644 --- a/ql/ql/test/queries/performance/VarUnusedInDisjunct/Test.qll +++ b/ql/ql/test/queries/performance/VarUnusedInDisjunct/Test.qll @@ -165,4 +165,6 @@ class HasField extends Big { or this.toString().matches("%foo") // <- field only defined here. } + + Big getField() { result = field } } diff --git a/ql/ql/test/queries/style/Misspelling/Misspelling.expected b/ql/ql/test/queries/style/Misspelling/Misspelling.expected index 8116e2b5afa7..1c02ca81d621 100644 --- a/ql/ql/test/queries/style/Misspelling/Misspelling.expected +++ b/ql/ql/test/queries/style/Misspelling/Misspelling.expected @@ -2,5 +2,5 @@ | Test.qll:4:7:4:26 | Class PublicallyAccessible | This class name contains the common misspelling 'publically', which should instead be 'publicly'. | | Test.qll:5:3:5:20 | FieldDecl | This field name contains the common misspelling 'occurences', which should instead be 'occurrences'. | | Test.qll:10:13:10:23 | ClassPredicate hasAgrument | This predicate name contains the common misspelling 'agrument', which should instead be 'argument'. | -| Test.qll:13:1:16:3 | QLDoc | This comment contains the non-US spelling 'colour', which should instead be 'color'. | -| Test.qll:17:7:17:17 | Class AnalysedInt | This class name contains the non-US spelling 'analysed', which should instead be 'analyzed'. | +| Test.qll:15:1:18:3 | QLDoc | This comment contains the non-US spelling 'colour', which should instead be 'color'. | +| Test.qll:19:7:19:17 | Class AnalysedInt | This class name contains the non-US spelling 'analysed', which should instead be 'analyzed'. | diff --git a/ql/ql/test/queries/style/Misspelling/Test.qll b/ql/ql/test/queries/style/Misspelling/Test.qll index f49f4633c6b8..b6619145f8d5 100644 --- a/ql/ql/test/queries/style/Misspelling/Test.qll +++ b/ql/ql/test/queries/style/Misspelling/Test.qll @@ -8,6 +8,8 @@ class PublicallyAccessible extends string { // should be argument predicate hasAgrument() { none() } + + int getNum() { result = numOccurences } } /** diff --git a/ql/ql/test/queries/style/UseInstanceofExtension/Foo.qll b/ql/ql/test/queries/style/UseInstanceofExtension/Foo.qll index 3429ce5b5994..b58cb3f93e37 100644 --- a/ql/ql/test/queries/style/UseInstanceofExtension/Foo.qll +++ b/ql/ql/test/queries/style/UseInstanceofExtension/Foo.qll @@ -22,6 +22,8 @@ class Inst3 extends string { Range range; Inst3() { this = range } + + Range getRange() { result = range } } class Inst4 extends string { diff --git a/ql/ql/test/queries/style/UseInstanceofExtension/UseInstanceofExtension.expected b/ql/ql/test/queries/style/UseInstanceofExtension/UseInstanceofExtension.expected index 72f110c632f9..804927fa0327 100644 --- a/ql/ql/test/queries/style/UseInstanceofExtension/UseInstanceofExtension.expected +++ b/ql/ql/test/queries/style/UseInstanceofExtension/UseInstanceofExtension.expected @@ -1,4 +1,4 @@ | Foo.qll:7:7:7:10 | Class Inst | Consider defining this class as non-extending subtype of $@. | Foo.qll:1:7:1:11 | Class Range | Range | | Foo.qll:15:7:15:11 | Class Inst2 | Consider defining this class as non-extending subtype of $@. | Foo.qll:1:7:1:11 | Class Range | Range | | Foo.qll:21:7:21:11 | Class Inst3 | Consider defining this class as non-extending subtype of $@. | Foo.qll:1:7:1:11 | Class Range | Range | -| Foo.qll:27:7:27:11 | Class Inst4 | Consider defining this class as non-extending subtype of $@. | Foo.qll:1:7:1:11 | Class Range | Range | +| Foo.qll:29:7:29:11 | Class Inst4 | Consider defining this class as non-extending subtype of $@. | Foo.qll:1:7:1:11 | Class Range | Range | diff --git a/ql/ql/test/queries/style/UseSetLiteral/UseSetLiteral.expected b/ql/ql/test/queries/style/UseSetLiteral/UseSetLiteral.expected index fac79ff078ed..ed17f5e1f1a7 100644 --- a/ql/ql/test/queries/style/UseSetLiteral/UseSetLiteral.expected +++ b/ql/ql/test/queries/style/UseSetLiteral/UseSetLiteral.expected @@ -4,5 +4,5 @@ | test.qll:62:7:65:14 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. | | test.qll:68:7:71:13 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. | | test.qll:74:7:77:13 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. | -| test.qll:87:3:90:9 | Disjunction | This formula of 4 predicate calls can be replaced with a single call on a set literal, improving readability. | -| test.qll:128:3:134:3 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. | +| test.qll:89:3:92:9 | Disjunction | This formula of 4 predicate calls can be replaced with a single call on a set literal, improving readability. | +| test.qll:130:3:136:3 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. | diff --git a/ql/ql/test/queries/style/UseSetLiteral/test.qll b/ql/ql/test/queries/style/UseSetLiteral/test.qll index 36a5f938f895..fcc581c3e8cd 100644 --- a/ql/ql/test/queries/style/UseSetLiteral/test.qll +++ b/ql/ql/test/queries/style/UseSetLiteral/test.qll @@ -81,6 +81,8 @@ class MyTest8Class extends int { predicate is(int x) { x = this } int get() { result = this } + + string getS() { result = s } } predicate test9(MyTest8Class c) {