Skip to content

Commit fe2988a

Browse files
authored
Merge pull request #2152 from yh-semmle/java-alert-suppression-annotations
Java: support LGTM alert suppression using `@SuppressWarnings` annotations
2 parents 7dd7463 + 155d14a commit fe2988a

File tree

7 files changed

+146
-20
lines changed

7 files changed

+146
-20
lines changed

java/config/suites/lgtm/java-queries-lgtm

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
@_namespace com.lgtm/java-queries
33
+ semmlecode-queries/AlertSuppression.ql
44
@_namespace com.lgtm/java-queries
5+
+ semmlecode-queries/AlertSuppressionAnnotations.ql
6+
@_namespace com.lgtm/java-queries
57
+ semmlecode-queries/filters/ClassifyFiles.ql
68
@_namespace com.lgtm/java-queries
79

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
/**
2+
* @name Alert suppression using annotations
3+
* @description Generates information about alert suppressions
4+
* using 'SuppressWarnings' annotations.
5+
* @kind alert-suppression
6+
* @id java/alert-suppression-annotations
7+
*/
8+
9+
import java
10+
import Metrics.Internal.Extents
11+
12+
/** Gets the LGTM suppression annotation text in the string `s`, if any. */
13+
bindingset[s]
14+
string getAnnotationText(string s) {
15+
// match `lgtm[...]` anywhere in the comment
16+
result = s.regexpFind("(?i)\\blgtm\\s*\\[[^\\]]*\\]", _, _)
17+
}
18+
19+
/**
20+
* An alert suppression annotation.
21+
*/
22+
class SuppressionAnnotation extends SuppressWarningsAnnotation {
23+
string text;
24+
25+
SuppressionAnnotation() {
26+
text = this.getASuppressedWarningLiteral().getValue() and
27+
exists(getAnnotationText(text))
28+
}
29+
30+
/**
31+
* Gets the text of this suppression annotation.
32+
*/
33+
string getText() { result = text }
34+
35+
private Annotation getASiblingAnnotation() {
36+
result = getAnnotatedElement().(Annotatable).getAnAnnotation() and
37+
(getAnnotatedElement() instanceof Callable or getAnnotatedElement() instanceof RefType)
38+
}
39+
40+
private Annotation firstAnnotation() {
41+
result = min(this.getASiblingAnnotation() as m
42+
order by
43+
m.getLocation().getStartLine(), m.getLocation().getStartColumn()
44+
)
45+
}
46+
47+
/**
48+
* Holds if this annotation applies to the range from column `startcolumn` of line `startline`
49+
* to column `endcolumn` of line `endline` in file `filepath`.
50+
*/
51+
predicate covers(string filepath, int startline, int startcolumn, int endline, int endcolumn) {
52+
if firstAnnotation().hasLocationInfo(filepath, _, _, _, _)
53+
then
54+
getAnnotatedElement().hasLocationInfo(filepath, _, _, endline, endcolumn) and
55+
firstAnnotation().hasLocationInfo(filepath, startline, startcolumn, _, _)
56+
else getAnnotatedElement().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
57+
}
58+
59+
/** Gets the scope of this suppression. */
60+
SuppressionScope getScope() { this = result.getSuppressionAnnotation() }
61+
}
62+
63+
/**
64+
* The scope of an alert suppression annotation.
65+
*/
66+
class SuppressionScope extends @annotation {
67+
SuppressionScope() { this instanceof SuppressionAnnotation }
68+
69+
/** Gets a suppression annotation with this scope. */
70+
SuppressionAnnotation getSuppressionAnnotation() { result = this }
71+
72+
/**
73+
* Holds if this element is at the specified location.
74+
* The location spans column `startcolumn` of line `startline` to
75+
* column `endcolumn` of line `endline` in file `filepath`.
76+
* For more information, see
77+
* [Locations](https://help.semmle.com/QL/learn-ql/ql/locations.html).
78+
*/
79+
predicate hasLocationInfo(
80+
string filepath, int startline, int startcolumn, int endline, int endcolumn
81+
) {
82+
this.(SuppressionAnnotation).covers(filepath, startline, startcolumn, endline, endcolumn)
83+
}
84+
85+
/** Gets a textual representation of this element. */
86+
string toString() { result = "suppression range" }
87+
}
88+
89+
from SuppressionAnnotation c, string text, string annotationText
90+
where
91+
text = c.getText() and
92+
annotationText = getAnnotationText(text)
93+
select c, // suppression entity
94+
text, // full text of suppression string
95+
annotationText, // LGTM suppression annotation text
96+
c.getScope() // scope of suppression

java/ql/src/Metrics/Internal/Extents.qll

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,19 @@
1-
import java
2-
3-
/*
1+
/**
2+
* Provides classes that modify the default location information
3+
* of `RefType`s and `Callable`s to cover their full extent.
4+
*
45
* When this library is imported, the `hasLocationInfo` predicate of
56
* `Callable` and `RefTypes` is overridden to specify their entire range
67
* instead of just the range of their name. The latter can still be
78
* obtained by invoking the `getLocation()` predicate.
89
*
9-
* The full ranges are required for the purpose of associating a violation
10-
* with an individual `Callable` or `RefType` as opposed to a whole `File`.
10+
* The full ranges may be used to determine whether a given location
11+
* (for example, the location of an alert) is contained within the extent
12+
* of a `Callable` or `RefType`.
1113
*/
1214

15+
import java
16+
1317
/**
1418
* A Callable whose `hasLocationInfo` is overridden to specify its entire range
1519
* including the body (if any), as opposed to the location of its name only.
@@ -48,20 +52,9 @@ class RangeRefType extends RefType {
4852
}
4953

5054
private Member lastMember() {
51-
exists(Member m, int i |
52-
result = m and
53-
m = getAMember() and
54-
i = rankOfMember(m) and
55-
not exists(Member other | other = getAMember() and rankOfMember(other) > i)
56-
)
57-
}
58-
59-
private int rankOfMember(Member m) {
60-
this.getAMember() = m and
61-
exists(Location mLoc, File f, int maxCol | mLoc = m.getLocation() |
62-
f = mLoc.getFile() and
63-
maxCol = max(Location loc | loc.getFile() = f | loc.getStartColumn()) and
64-
result = mLoc.getStartLine() * maxCol + mLoc.getStartColumn()
65-
)
55+
result = max(this.getAMember() as m
56+
order by
57+
m.getLocation().getStartLine(), m.getLocation().getStartColumn()
58+
)
6659
}
6760
}

java/ql/src/semmle/code/java/JDKAnnotations.qll

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ class OverrideAnnotation extends Annotation {
1818
class SuppressWarningsAnnotation extends Annotation {
1919
SuppressWarningsAnnotation() { this.getType().hasQualifiedName("java.lang", "SuppressWarnings") }
2020

21+
/** Gets the `StringLiteral` of a warning suppressed by this annotation. */
22+
StringLiteral getASuppressedWarningLiteral() {
23+
result = this.getAValue() or
24+
result = this.getAValue().(ArrayInit).getAnInit()
25+
}
26+
2127
/** Gets the name of a warning suppressed by this annotation. */
2228
string getASuppressedWarning() {
2329
result = this.getAValue().(StringLiteral).getLiteral() or
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
| TestSuppressWarnings.java:2:1:2:49 | SuppressWarnings | lgtm[java/non-sync-override] | lgtm[java/non-sync-override] | TestSuppressWarnings.java:2:1:21:5 | suppression range |
2+
| TestSuppressWarnings.java:5:5:5:31 | SuppressWarnings | lgtm[] | lgtm[] | TestSuppressWarnings.java:5:5:8:5 | suppression range |
3+
| TestSuppressWarnings.java:10:5:10:104 | SuppressWarnings | lgtm[java/confusing-method-name] not confusing | lgtm[java/confusing-method-name] | TestSuppressWarnings.java:9:5:13:5 | suppression range |
4+
| TestSuppressWarnings.java:10:5:10:104 | SuppressWarnings | lgtm[java/non-sync-override] | lgtm[java/non-sync-override] | TestSuppressWarnings.java:9:5:13:5 | suppression range |
5+
| TestSuppressWarnings.java:18:5:18:98 | SuppressWarnings | lgtm[java/confusing-method-name] blah blah lgtm[java/non-sync-override] | lgtm[java/confusing-method-name] | TestSuppressWarnings.java:18:5:21:5 | suppression range |
6+
| TestSuppressWarnings.java:18:5:18:98 | SuppressWarnings | lgtm[java/confusing-method-name] blah blah lgtm[java/non-sync-override] | lgtm[java/non-sync-override] | TestSuppressWarnings.java:18:5:21:5 | suppression range |
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
AlertSuppressionAnnotations.ql
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
2+
@SuppressWarnings("lgtm[java/non-sync-override]")
3+
@Deprecated
4+
class TestSuppressWarnings {
5+
@SuppressWarnings("lgtm[]")
6+
public void test() {
7+
8+
}
9+
@Deprecated
10+
@SuppressWarnings({"lgtm[java/confusing-method-name] not confusing","lgtm[java/non-sync-override]"})
11+
public void test2() {
12+
13+
}
14+
@SuppressWarnings("lgtm")
15+
public void test3() {
16+
17+
}
18+
@SuppressWarnings({"lgtm[java/confusing-method-name] blah blah lgtm[java/non-sync-override]"})
19+
public void test4() {
20+
21+
}
22+
}

0 commit comments

Comments
 (0)