From acac2032ddfea202b2f73e339dc46401537d5ad7 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 2 Jun 2025 16:07:47 +0200 Subject: [PATCH] JS: Update type usage in UnreachableMethodOverloads This query depended on the cons-hashing performed by type extraction to determine if two types are the same. This is not trivial to restore, but not important enough to reimplement right now, so for now just simplifying the query's ability to recognise that two types are the same. --- .../UnreachableMethodOverloads.ql | 40 ++++++++++++------- .../UnreachableMethodOverloads.expected | 3 -- .../Declarations/UnreachableOverloads/tst.ts | 6 +-- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/javascript/ql/src/Declarations/UnreachableMethodOverloads.ql b/javascript/ql/src/Declarations/UnreachableMethodOverloads.ql index a68617ea2f1..912d58ab54c 100644 --- a/javascript/ql/src/Declarations/UnreachableMethodOverloads.ql +++ b/javascript/ql/src/Declarations/UnreachableMethodOverloads.ql @@ -45,20 +45,22 @@ string getKind(MemberDeclaration m) { /** * A call-signature that originates from a MethodSignature in the AST. */ -private class MethodCallSig extends CallSignatureType { - string name; +private class MethodCallSig extends Function { + private MethodSignature signature; - MethodCallSig() { - exists(MethodSignature sig | - this = sig.getBody().getCallSignature() and - name = sig.getName() - ) + MethodCallSig() { this = signature.getBody() } + + int getNumOptionalParameter() { + result = count(Parameter p | p = this.getParameter(_) and p.isDeclaredOptional()) } - /** - * Gets the name of any member that has this signature. - */ - string getName() { result = name } + int getNumRequiredParameter() { + result = count(Parameter p | p = this.getParameter(_) and not p.isDeclaredOptional()) + } + + SignatureKind getKind() { result = SignatureKind::function() } + + TypeExpr getTypeParameterBound(int i) { result = this.getTypeParameter(i).getBound() } } pragma[noinline] @@ -75,6 +77,7 @@ private MethodCallSig getMethodCallSigWithFingerprint( /** * Holds if the two call signatures could be overloads of each other and have the same parameter types. */ +pragma[inline] predicate matchingCallSignature(MethodCallSig method, MethodCallSig other) { other = getMethodCallSigWithFingerprint(method.getName(), method.getNumOptionalParameter(), @@ -109,6 +112,16 @@ private MethodSignature getMethodSignatureWithFingerprint( result.getBody().getNumParameter() = numParameters } +bindingset[t1, t2] +pragma[inline_late] +private predicate sameType(TypeExpr t1, TypeExpr t2) { + t1.(PredefinedTypeExpr).getName() = t2.(PredefinedTypeExpr).getName() + or + t1 instanceof ThisTypeExpr and t2 instanceof ThisTypeExpr + or + t1.(LocalTypeAccess).getLocalTypeName() = t2.(LocalTypeAccess).getLocalTypeName() +} + /** * Holds if the two method signatures are overloads of each other and have the same parameter types. */ @@ -122,14 +135,13 @@ predicate signaturesMatch(MethodSignature method, MethodSignature other) { not exists(method.getBody().getThisTypeAnnotation()) and not exists(other.getBody().getThisTypeAnnotation()) or - method.getBody().getThisTypeAnnotation().getType() = - other.getBody().getThisTypeAnnotation().getType() + sameType(method.getBody().getThisTypeAnnotation(), other.getBody().getThisTypeAnnotation()) ) and // The types are compared in matchingCallSignature. This is a consistency check that the textual representation of the type-annotations are somewhat similar. forall(int i | i in [0 .. -1 + method.getBody().getNumParameter()] | getParameterTypeAnnotation(method, i) = getParameterTypeAnnotation(other, i) ) and - matchingCallSignature(method.getBody().getCallSignature(), other.getBody().getCallSignature()) + matchingCallSignature(method.getBody(), other.getBody()) } from ClassOrInterface decl, string name, MethodSignature previous, MethodSignature unreachable diff --git a/javascript/ql/test/query-tests/Declarations/UnreachableOverloads/UnreachableMethodOverloads.expected b/javascript/ql/test/query-tests/Declarations/UnreachableOverloads/UnreachableMethodOverloads.expected index 44bd568e767..24767950a66 100644 --- a/javascript/ql/test/query-tests/Declarations/UnreachableOverloads/UnreachableMethodOverloads.expected +++ b/javascript/ql/test/query-tests/Declarations/UnreachableOverloads/UnreachableMethodOverloads.expected @@ -1,5 +1,2 @@ -| tst.ts:3:3:3:30 | method( ... number; | This overload of method() is unreachable, the $@ overload will always be selected. | tst.ts:2:3:2:30 | method( ... string; | previous | | tst.ts:6:3:6:17 | types1(): any[] | This overload of types1() is unreachable, the $@ overload will always be selected. | tst.ts:5:3:5:18 | types1(): T[] | previous | -| tst.ts:15:3:15:74 | on(even ... nction; | This overload of on() is unreachable, the $@ overload will always be selected. | tst.ts:14:3:14:74 | on(even ... nction; | previous | | tst.ts:21:3:21:28 | bar(thi ... number; | This overload of bar() is unreachable, the $@ overload will always be selected. | tst.ts:20:3:20:28 | bar(thi ... string; | previous | -| tst.ts:27:3:27:30 | method( ... number; | This overload of method() is unreachable, the $@ overload will always be selected. | tst.ts:26:3:26:30 | method( ... string; | previous | diff --git a/javascript/ql/test/query-tests/Declarations/UnreachableOverloads/tst.ts b/javascript/ql/test/query-tests/Declarations/UnreachableOverloads/tst.ts index 17d95f835cf..4ffd64bc7c1 100644 --- a/javascript/ql/test/query-tests/Declarations/UnreachableOverloads/tst.ts +++ b/javascript/ql/test/query-tests/Declarations/UnreachableOverloads/tst.ts @@ -1,6 +1,6 @@ declare class Foobar { method(foo: number): string; - method(foo: number): number; // $ Alert + method(foo: number): number; // $ MISSING: Alert types1(): T[] types1(): any[] // $ Alert @@ -12,7 +12,7 @@ declare class Foobar { types3(t: T): number on(event: string, fn?: (event?: any, ...args: any[]) => void): Function; - on(event: string, fn?: (event?: any, ...args: any[]) => void): Function; // $ Alert + on(event: string, fn?: (event?: any, ...args: any[]) => void): Function; // $ MISSING: Alert foo(this: string): string; foo(this: number): number; @@ -24,7 +24,7 @@ declare class Foobar { declare class Base { method(foo: number): string; - method(foo: number): number; // $ Alert + method(foo: number): number; // $ MISSING: Alert overRiddenInSub(): string; overRiddenInSub(): number;