Skip to content

Commit 1a7ddfb

Browse files
committed
Improve incomplete-sequence-of-chars.ql
Removed checks for missing chars at end of string because that caused too many false positives. Tested query with Variant Analysis and had a few results, but also a few false positives for example for hardcoded hash value strings.
1 parent 6a14272 commit 1a7ddfb

File tree

1 file changed

+22
-59
lines changed

1 file changed

+22
-59
lines changed

codeql-custom-queries-java/not-tested-queries/incomplete-sequence-of-chars.ql renamed to codeql-custom-queries-java/queries/likely-bugs/incomplete-sequence-of-chars.ql

Lines changed: 22 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
* // Is missing the 4
55
* String digits = "123567890";
66
* ```
7+
*
8+
* @kind problem
79
*/
810

911
import java
@@ -74,22 +76,14 @@ string getSubsequentChar(string first) {
7476
if (first.isLowercase()) then (
7577
areConsecutiveCharsLower(first, result)
7678
) else exists (string secondLower |
77-
areConsecutiveCharsLower(first.toLowerCase(), secondLower)
78-
and result = secondLower.toUpperCase()
79+
areConsecutiveCharsLower(first.toLowerCase(), secondLower)
80+
and result = secondLower.toUpperCase()
7981
)
8082
}
8183

82-
predicate isAllowedLastChar(string last) {
83-
last = [
84-
"f", // Last hex digit
85-
"z",
86-
"0",
87-
"9"
88-
]
89-
}
90-
9184
predicate hasSequenceStart(IncompleteSequenceExpr expr) {
92-
// Check first 3 chars, otherwise causes too much negatives, e.g. for "delete", "no", ...
85+
// Check first 3 chars, otherwise causes a lot of false positives in the middle
86+
// of arbitrary strings
9387
exists (string first, string second, string third |
9488
first = expr.charAt(0)
9589
and second = expr.charAt(1)
@@ -98,56 +92,35 @@ predicate hasSequenceStart(IncompleteSequenceExpr expr) {
9892
areConsecutiveChars(first, second)
9993
and areConsecutiveChars(second, third)
10094
)
101-
// Exclude words containing three or more consecutive chars
102-
and not expr.getString().toLowerCase().matches([
103-
"default%",
104-
"stub%"
105-
])
10695
}
10796

10897
abstract class IncompleteSequenceExpr extends Expr {
10998
abstract string getString();
11099
abstract string charAt(int index);
111-
abstract int length();
112100

113101
string getMissingChar(int index) {
114-
exists (string previousCorrect, string lastCorrect |
102+
exists (string previousCorrect, string lastCorrect, string incorrect |
115103
previousCorrect = charAt(index - 2)
116104
and lastCorrect = charAt(index - 1)
105+
and incorrect = charAt(index)
117106
// Make sure that there is a consecutive char in front, otherwise
118107
// sequence which then later has other chars causes false positives
119-
// e.g. "abc-15-15" would find matches for "15" (-> "12")
108+
// e.g. "abc-15" would find matches for "15" (-> "12")
120109
and areConsecutiveChars(previousCorrect, lastCorrect)
121110
and result = getSubsequentChar(lastCorrect)
122111
|
123-
exists (string incorrect |
124-
incorrect = charAt(index)
125-
|
126-
// Verify that both chars are of the same type; ignore somthing like "12.0", "ABC-123"
127-
areSameType(lastCorrect, incorrect)
128-
and not areConsecutiveChars(lastCorrect, incorrect)
129-
and (
130-
index = length() - 1
131-
// Char was skipped, e.g. "abcd_f"
132-
or areConsecutiveChars(result, incorrect)
133-
// Duplicate char
134-
or (
135-
index <= length() - 1 // At least one more char
136-
and lastCorrect = incorrect
137-
and areConsecutiveChars(lastCorrect, charAt(index + 1))
138-
)
112+
// Verify that both chars are of the same type; ignore something like "12.0", "ABC-123"
113+
areSameType(lastCorrect, incorrect)
114+
and not areConsecutiveChars(lastCorrect, incorrect)
115+
and (
116+
// Char was skipped, e.g. "abcd_f"
117+
areConsecutiveChars(result, incorrect)
118+
// Duplicate char
119+
or (
120+
lastCorrect = incorrect
121+
and areConsecutiveChars(lastCorrect, charAt(index + 1))
139122
)
140123
)
141-
or (
142-
(
143-
// Last char
144-
index = length()
145-
// Or next char has different type
146-
or not areSameType(lastCorrect, charAt(index))
147-
)
148-
// Verify that sequence is not allowed to end with `lastCorrect`
149-
and not isAllowedLastChar(lastCorrect.toLowerCase())
150-
)
151124
)
152125
}
153126
}
@@ -162,11 +135,6 @@ class IncompleteStringLiteralExpr extends IncompleteSequenceExpr, StringLiteral
162135
string charAt(int index) {
163136
result = getString().charAt(index)
164137
}
165-
166-
override
167-
int length() {
168-
result = getString().length()
169-
}
170138
}
171139

172140
class IncompleteCharArrayExpr extends IncompleteSequenceExpr, ArrayInit {
@@ -179,16 +147,11 @@ class IncompleteCharArrayExpr extends IncompleteSequenceExpr, ArrayInit {
179147
string charAt(int index) {
180148
result = getInit(index).(CharacterLiteral).getValue()
181149
}
182-
183-
override
184-
int length() {
185-
// Count elements of initializer
186-
result = count(getAnInit())
187-
}
188150
}
189151

190-
from IncompleteSequenceExpr incompleteSeq, int index
152+
from IncompleteSequenceExpr incompleteSeq, int index, string missingChar
191153
where
192154
hasSequenceStart(incompleteSeq)
155+
and missingChar = incompleteSeq.getMissingChar(index)
193156
and not incompleteSeq.getEnclosingCallable().getDeclaringType() instanceof TestClass
194-
select incompleteSeq, "Sequence of chars is incomplete, is missing '" + incompleteSeq.getMissingChar(index) + "' at index " + index
157+
select incompleteSeq, "Sequence of chars is incomplete, is missing '" + missingChar + "' at index " + index

0 commit comments

Comments
 (0)