From 431aa2cb7967720aa867987fa90d665467b9d6c1 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Tue, 20 Sep 2022 22:34:53 +0200 Subject: [PATCH 1/3] Java: Add `CompilationUnit.getATypeAvailableBySimpleName()` This predicate is mainly helpful for Javadoc queries and for queries which check whether the name of an element shadows another type. --- ...-09-20-CompilationUnit-simple-name-type.md | 4 ++ .../lib/semmle/code/java/CompilationUnit.qll | 45 +++++++++++++++++++ .../Documentation/ImpossibleJavadocThrows.ql | 14 +++--- 3 files changed, 54 insertions(+), 9 deletions(-) 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-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 9b4b58e9a9bd..621750b90986 100644 --- a/java/ql/lib/semmle/code/java/CompilationUnit.qll +++ b/java/ql/lib/semmle/code/java/CompilationUnit.qll @@ -31,5 +31,50 @@ 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 7e2738af1e1b..1a5862bb9e83 100644 --- a/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql +++ b/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql @@ -11,22 +11,18 @@ import java -RefType getTaggedType(ThrowsTag tag) { +Class getTaggedType(ThrowsTag tag) { result.hasName(tag.getExceptionName()) and - exists(ImportType i | i.getFile() = tag.getFile() | i.getImportedType() = result) + result = tag.getFile().(CompilationUnit).getATypeAvailableBySimpleName() } -predicate canThrow(Callable callable, RefType exception) { - exists(string uncheckedException | - uncheckedException = "RuntimeException" or uncheckedException = "Error" - | - exception.getAnAncestor().hasQualifiedName("java.lang", uncheckedException) - ) +predicate canThrow(Callable callable, Class exception) { + exception instanceof UncheckedThrowableType or callable.getAnException().getType().getADescendant() = exception } -from ThrowsTag throwsTag, RefType thrownType, Callable docMethod +from ThrowsTag throwsTag, Class thrownType, Callable docMethod where getTaggedType(throwsTag) = thrownType and docMethod.getDoc().getJavadoc().getAChild*() = throwsTag and From fd99ae78b32612d6b469430811d9a97cf30e862b Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Sun, 25 Sep 2022 14:44:16 +0200 Subject: [PATCH 2/3] Java: Rename predicate to `getATypeInScope` --- ...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, 25 insertions(+), 37 deletions(-) create mode 100644 java/ql/lib/change-notes/2022-09-20-CompilationUnit-getATypeInScope.md 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-getATypeInScope.md b/java/ql/lib/change-notes/2022-09-20-CompilationUnit-getATypeInScope.md new file mode 100644 index 000000000000..dfb7e9390601 --- /dev/null +++ b/java/ql/lib/change-notes/2022-09-20-CompilationUnit-getATypeInScope.md @@ -0,0 +1,4 @@ +--- +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 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..74396e7086b4 100644 --- a/java/ql/lib/semmle/code/java/CompilationUnit.qll +++ b/java/ql/lib/semmle/code/java/CompilationUnit.qll @@ -32,42 +32,30 @@ 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` + * 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. */ - ClassOrInterface getATypeAvailableBySimpleName() { + ClassOrInterface getATypeInScope() { // 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 + // Currently shadowing is not considered 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() + exists(Import importDecl | importDecl.getCompilationUnit() = this | + result = + [ + importDecl.(ImportStaticTypeMember).getATypeImport(), + importDecl.(ImportType).getImportedType(), + importDecl.(ImportStaticOnDemand).getATypeImport(), + importDecl.(ImportOnDemandFromType).getAnImport(), + importDecl.(ImportOnDemandFromPackage).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 1a5862bb9e83..2452dda4fc45 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).getATypeAvailableBySimpleName() + result = tag.getFile().(CompilationUnit).getATypeInScope() } predicate canThrow(Callable callable, Class exception) { From c40b6285a29563ea812b7496de0f1a45aea8935a Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Mon, 26 Sep 2022 12:08:43 +0200 Subject: [PATCH 3/3] Java: Adjust `ImpossibleJavadocThrows.ql` --- .../src/Advisory/Documentation/ImpossibleJavadocThrows.ql | 6 ++++-- .../query-tests/Javadoc/ImpossibleJavadocThrows.expected | 1 + .../test/query-tests/Javadoc/ImpossibleJavadocThrows.java | 7 +++++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql b/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql index 2452dda4fc45..3111c704b4d3 100644 --- a/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql +++ b/java/ql/src/Advisory/Documentation/ImpossibleJavadocThrows.ql @@ -11,7 +11,7 @@ import java -Class getTaggedType(ThrowsTag tag) { +ClassOrInterface getTaggedType(ThrowsTag tag) { result.hasName(tag.getExceptionName()) and result = tag.getFile().(CompilationUnit).getATypeInScope() } @@ -22,7 +22,9 @@ predicate canThrow(Callable callable, Class exception) { callable.getAnException().getType().getADescendant() = exception } -from ThrowsTag throwsTag, Class 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 diff --git a/java/ql/test/query-tests/Javadoc/ImpossibleJavadocThrows.expected b/java/ql/test/query-tests/Javadoc/ImpossibleJavadocThrows.expected index f94a6f61f33a..5b031604e567 100644 --- a/java/ql/test/query-tests/Javadoc/ImpossibleJavadocThrows.expected +++ b/java/ql/test/query-tests/Javadoc/ImpossibleJavadocThrows.expected @@ -1,2 +1,3 @@ | 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 7ba8988c38be..b0b27397359b 100644 --- a/java/ql/test/query-tests/Javadoc/ImpossibleJavadocThrows.java +++ b/java/ql/test/query-tests/Javadoc/ImpossibleJavadocThrows.java @@ -18,6 +18,13 @@ public void bad1() { public void bad2() { } + /** + * + * @throws Runnable + */ + public void bad3() { + } + /** * * @throws InterruptedException