From c1654ce7cc1d57e80054827800a184b33c2ef68e Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Tue, 4 Oct 2022 10:56:32 +0200 Subject: [PATCH 1/5] Revert "Java: Fix cartesian product" --- .../Advisory/Documentation/ImpossibleJavadocThrows.ql | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql b/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql index 42fd2049289c..3111c704b4d3 100644 --- a/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql +++ b/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql @@ -16,14 +16,19 @@ ClassOrInterface getTaggedType(ThrowsTag tag) { result = tag.getFile().(CompilationUnit).getATypeInScope() } +predicate canThrow(Callable callable, Class exception) { + exception instanceof UncheckedThrowableType + or + callable.getAnException().getType().getADescendant() = exception +} + // Uses ClassOrInterface as type for thrownType to also cover case where erroneously an interface // type is declared as thrown exception from ThrowsTag throwsTag, ClassOrInterface thrownType, Callable docMethod where getTaggedType(throwsTag) = thrownType and docMethod.getDoc().getJavadoc().getAChild*() = throwsTag and - not thrownType instanceof UncheckedThrowableType and - not docMethod.getAnException().getType().getADescendant() = thrownType + not canThrow(docMethod, thrownType) select throwsTag, "Javadoc for " + docMethod + " claims to throw " + thrownType.getName() + " but this is impossible." From df29e05b9f1778c0604b6916a591bd8d502b588d Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Tue, 4 Oct 2022 10:59:39 +0200 Subject: [PATCH 2/5] Revert "Java: Adjust `ImpossibleJavadocThrows.ql`" This reverts commit c40b6285a29563ea812b7496de0f1a45aea8935a. --- .../src/Advisory/Documentation/ImpossibleJavadocThrows.ql | 6 ++---- .../query-tests/Javadoc/ImpossibleJavadocThrows.expected | 1 - .../test/query-tests/Javadoc/ImpossibleJavadocThrows.java | 7 ------- 3 files changed, 2 insertions(+), 12 deletions(-) diff --git a/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql b/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql index 3111c704b4d3..2452dda4fc45 100644 --- a/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql +++ b/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql @@ -11,7 +11,7 @@ import java -ClassOrInterface getTaggedType(ThrowsTag tag) { +Class getTaggedType(ThrowsTag tag) { result.hasName(tag.getExceptionName()) and result = tag.getFile().(CompilationUnit).getATypeInScope() } @@ -22,9 +22,7 @@ predicate canThrow(Callable callable, Class exception) { callable.getAnException().getType().getADescendant() = exception } -// Uses ClassOrInterface as type for thrownType to also cover case where erroneously an interface -// type is declared as thrown exception -from ThrowsTag throwsTag, ClassOrInterface thrownType, Callable docMethod +from ThrowsTag throwsTag, Class thrownType, Callable docMethod where getTaggedType(throwsTag) = thrownType and docMethod.getDoc().getJavadoc().getAChild*() = throwsTag and diff --git a/java/ql/test/query-tests/Javadoc/ImpossibleJavadocThrows.expected b/java/ql/test/query-tests/Javadoc/ImpossibleJavadocThrows.expected index 5b031604e567..f94a6f61f33a 100644 --- a/java/ql/test/query-tests/Javadoc/ImpossibleJavadocThrows.expected +++ b/java/ql/test/query-tests/Javadoc/ImpossibleJavadocThrows.expected @@ -1,3 +1,2 @@ | ImpossibleJavadocThrows.java:9:5:9:12 | @throws | Javadoc for bad1 claims to throw InterruptedException but this is impossible. | | ImpossibleJavadocThrows.java:16:5:16:15 | @exception | Javadoc for bad2 claims to throw Exception but this is impossible. | -| ImpossibleJavadocThrows.java:23:5:23:12 | @throws | Javadoc for bad3 claims to throw Runnable but this is impossible. | diff --git a/java/ql/test/query-tests/Javadoc/ImpossibleJavadocThrows.java b/java/ql/test/query-tests/Javadoc/ImpossibleJavadocThrows.java index b0b27397359b..7ba8988c38be 100644 --- a/java/ql/test/query-tests/Javadoc/ImpossibleJavadocThrows.java +++ b/java/ql/test/query-tests/Javadoc/ImpossibleJavadocThrows.java @@ -18,13 +18,6 @@ public void bad1() { public void bad2() { } - /** - * - * @throws Runnable - */ - public void bad3() { - } - /** * * @throws InterruptedException From 01b950f68b4119f0fa95e6810da8789b33bf7cb5 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Tue, 4 Oct 2022 10:59:43 +0200 Subject: [PATCH 3/5] Revert "Java: Rename predicate to `getATypeInScope`" This reverts commit fd99ae78b32612d6b469430811d9a97cf30e862b. --- ...2-09-20-CompilationUnit-getATypeInScope.md | 4 -- ...-09-20-CompilationUnit-simple-name-type.md | 4 ++ .../lib/semmle/code/java/CompilationUnit.qll | 52 ++++++++++++------- .../Documentation/ImpossibleJavadocThrows.ql | 2 +- 4 files changed, 37 insertions(+), 25 deletions(-) delete mode 100644 java/ql/lib/change-notes/2022-09-20-CompilationUnit-getATypeInScope.md create mode 100644 java/ql/lib/change-notes/2022-09-20-CompilationUnit-simple-name-type.md diff --git a/java/ql/lib/change-notes/2022-09-20-CompilationUnit-getATypeInScope.md b/java/ql/lib/change-notes/2022-09-20-CompilationUnit-getATypeInScope.md deleted file mode 100644 index dfb7e9390601..000000000000 --- a/java/ql/lib/change-notes/2022-09-20-CompilationUnit-getATypeInScope.md +++ /dev/null @@ -1,4 +0,0 @@ ---- -category: feature ---- -* Added the predicate `CompilationUnit.getATypeInScope()`. diff --git a/java/ql/lib/change-notes/2022-09-20-CompilationUnit-simple-name-type.md b/java/ql/lib/change-notes/2022-09-20-CompilationUnit-simple-name-type.md new file mode 100644 index 000000000000..1ce21f6e1809 --- /dev/null +++ b/java/ql/lib/change-notes/2022-09-20-CompilationUnit-simple-name-type.md @@ -0,0 +1,4 @@ +--- +category: feature +--- +* Added the predicate `CompilationUnit.getATypeAvailableBySimpleName()`. diff --git a/java/ql/lib/semmle/code/java/CompilationUnit.qll b/java/ql/lib/semmle/code/java/CompilationUnit.qll index 74396e7086b4..621750b90986 100644 --- a/java/ql/lib/semmle/code/java/CompilationUnit.qll +++ b/java/ql/lib/semmle/code/java/CompilationUnit.qll @@ -32,30 +32,42 @@ class CompilationUnit extends Element, File { Module getModule() { cumodule(this, result) } /** - * Gets a type which is available in the top-level scope of this compilation unit. - * This can be a type: - * - declared in this compilation unit as top-level type - * - imported with an `import` declaration - * - declared in the same package as this compilation unit - * - declared in the package `java.lang` - * - * This predicate not consider "shadowing", it can have types as result whose simple name is - * shadowed by another type in scope. + * Gets a type which is available by its simple name in this compilation unit. + * Reasons for this can be: + * - The type is declared in this compilation unit as top-level type + * - The type is imported + * - The type is declared in the same package as this compilation unit + * - The type is declared in the package `java.lang` */ - ClassOrInterface getATypeInScope() { + ClassOrInterface getATypeAvailableBySimpleName() { // See "Shadowing", https://docs.oracle.com/javase/specs/jls/se17/html/jls-6.html#jls-6.4.1 - // Currently shadowing is not considered + // Note: Currently the logic below does not consider shadowing and might have multiple results + // with the same type name result.(TopLevelType).getCompilationUnit() = this or - exists(Import importDecl | importDecl.getCompilationUnit() = this | - result = - [ - importDecl.(ImportStaticTypeMember).getATypeImport(), - importDecl.(ImportType).getImportedType(), - importDecl.(ImportStaticOnDemand).getATypeImport(), - importDecl.(ImportOnDemandFromType).getAnImport(), - importDecl.(ImportOnDemandFromPackage).getAnImport(), - ] + exists(ImportStaticTypeMember importDecl | + importDecl.getCompilationUnit() = this and + result = importDecl.getATypeImport() + ) + or + exists(ImportType importDecl | + importDecl.getCompilationUnit() = this and + result = importDecl.getImportedType() + ) + or + exists(ImportStaticOnDemand importDecl | + importDecl.getCompilationUnit() = this and + result = importDecl.getATypeImport() + ) + or + exists(ImportOnDemandFromType importDecl | + importDecl.getCompilationUnit() = this and + result = importDecl.getAnImport() + ) + or + exists(ImportOnDemandFromPackage importDecl | + importDecl.getCompilationUnit() = this and + result = importDecl.getAnImport() ) or // From same package or java.lang, see https://docs.oracle.com/javase/specs/jls/se17/html/jls-7.html diff --git a/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql b/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql index 2452dda4fc45..1a5862bb9e83 100644 --- a/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql +++ b/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql @@ -13,7 +13,7 @@ import java Class getTaggedType(ThrowsTag tag) { result.hasName(tag.getExceptionName()) and - result = tag.getFile().(CompilationUnit).getATypeInScope() + result = tag.getFile().(CompilationUnit).getATypeAvailableBySimpleName() } predicate canThrow(Callable callable, Class exception) { From 281e49daf7fc2fbfa3afe7c6841e16d81b5d8194 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Tue, 4 Oct 2022 10:59:45 +0200 Subject: [PATCH 4/5] Revert "Java: Add `CompilationUnit.getATypeAvailableBySimpleName()`" This reverts commit 431aa2cb7967720aa867987fa90d665467b9d6c1. --- ...-09-20-CompilationUnit-simple-name-type.md | 4 -- .../lib/semmle/code/java/CompilationUnit.qll | 45 ------------------- .../Documentation/ImpossibleJavadocThrows.ql | 14 +++--- 3 files changed, 9 insertions(+), 54 deletions(-) delete mode 100644 java/ql/lib/change-notes/2022-09-20-CompilationUnit-simple-name-type.md diff --git a/java/ql/lib/change-notes/2022-09-20-CompilationUnit-simple-name-type.md b/java/ql/lib/change-notes/2022-09-20-CompilationUnit-simple-name-type.md deleted file mode 100644 index 1ce21f6e1809..000000000000 --- a/java/ql/lib/change-notes/2022-09-20-CompilationUnit-simple-name-type.md +++ /dev/null @@ -1,4 +0,0 @@ ---- -category: feature ---- -* Added the predicate `CompilationUnit.getATypeAvailableBySimpleName()`. diff --git a/java/ql/lib/semmle/code/java/CompilationUnit.qll b/java/ql/lib/semmle/code/java/CompilationUnit.qll index 621750b90986..9b4b58e9a9bd 100644 --- a/java/ql/lib/semmle/code/java/CompilationUnit.qll +++ b/java/ql/lib/semmle/code/java/CompilationUnit.qll @@ -31,50 +31,5 @@ class CompilationUnit extends Element, File { */ Module getModule() { cumodule(this, result) } - /** - * Gets a type which is available by its simple name in this compilation unit. - * Reasons for this can be: - * - The type is declared in this compilation unit as top-level type - * - The type is imported - * - The type is declared in the same package as this compilation unit - * - The type is declared in the package `java.lang` - */ - ClassOrInterface getATypeAvailableBySimpleName() { - // See "Shadowing", https://docs.oracle.com/javase/specs/jls/se17/html/jls-6.html#jls-6.4.1 - // Note: Currently the logic below does not consider shadowing and might have multiple results - // with the same type name - result.(TopLevelType).getCompilationUnit() = this - or - exists(ImportStaticTypeMember importDecl | - importDecl.getCompilationUnit() = this and - result = importDecl.getATypeImport() - ) - or - exists(ImportType importDecl | - importDecl.getCompilationUnit() = this and - result = importDecl.getImportedType() - ) - or - exists(ImportStaticOnDemand importDecl | - importDecl.getCompilationUnit() = this and - result = importDecl.getATypeImport() - ) - or - exists(ImportOnDemandFromType importDecl | - importDecl.getCompilationUnit() = this and - result = importDecl.getAnImport() - ) - or - exists(ImportOnDemandFromPackage importDecl | - importDecl.getCompilationUnit() = this and - result = importDecl.getAnImport() - ) - or - // From same package or java.lang, see https://docs.oracle.com/javase/specs/jls/se17/html/jls-7.html - result.(TopLevelType).getPackage() = this.getPackage() - or - result.(TopLevelType).getPackage().hasName("java.lang") - } - override string getAPrimaryQlClass() { result = "CompilationUnit" } } diff --git a/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql b/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql index 1a5862bb9e83..7e2738af1e1b 100644 --- a/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql +++ b/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql @@ -11,18 +11,22 @@ import java -Class getTaggedType(ThrowsTag tag) { +RefType getTaggedType(ThrowsTag tag) { result.hasName(tag.getExceptionName()) and - result = tag.getFile().(CompilationUnit).getATypeAvailableBySimpleName() + exists(ImportType i | i.getFile() = tag.getFile() | i.getImportedType() = result) } -predicate canThrow(Callable callable, Class exception) { - exception instanceof UncheckedThrowableType +predicate canThrow(Callable callable, RefType exception) { + exists(string uncheckedException | + uncheckedException = "RuntimeException" or uncheckedException = "Error" + | + exception.getAnAncestor().hasQualifiedName("java.lang", uncheckedException) + ) or callable.getAnException().getType().getADescendant() = exception } -from ThrowsTag throwsTag, Class thrownType, Callable docMethod +from ThrowsTag throwsTag, RefType thrownType, Callable docMethod where getTaggedType(throwsTag) = thrownType and docMethod.getDoc().getJavadoc().getAChild*() = throwsTag and From 2deb3e56251e6fe2eec9a26a379ac1595b7609a4 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Tue, 4 Oct 2022 11:11:44 +0200 Subject: [PATCH 5/5] Reapply "Java: Fix cartesian product" This reverts commit c1654ce7cc1d57e80054827800a184b33c2ef68e. --- .../Documentation/ImpossibleJavadocThrows.ql | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql b/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql index 7e2738af1e1b..7f8f4d4f983d 100644 --- a/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql +++ b/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql @@ -16,21 +16,14 @@ RefType getTaggedType(ThrowsTag tag) { exists(ImportType i | i.getFile() = tag.getFile() | i.getImportedType() = result) } -predicate canThrow(Callable callable, RefType exception) { - exists(string uncheckedException | - uncheckedException = "RuntimeException" or uncheckedException = "Error" - | - exception.getAnAncestor().hasQualifiedName("java.lang", uncheckedException) - ) - or - callable.getAnException().getType().getADescendant() = exception -} - -from ThrowsTag throwsTag, RefType thrownType, Callable docMethod +// Uses ClassOrInterface as type for thrownType to also cover case where erroneously an interface +// type is declared as thrown exception +from ThrowsTag throwsTag, ClassOrInterface thrownType, Callable docMethod where getTaggedType(throwsTag) = thrownType and docMethod.getDoc().getJavadoc().getAChild*() = throwsTag and - not canThrow(docMethod, thrownType) + not thrownType instanceof UncheckedThrowableType and + not docMethod.getAnException().getType().getADescendant() = thrownType select throwsTag, "Javadoc for " + docMethod + " claims to throw " + thrownType.getName() + " but this is impossible."