Skip to content

JS: Fix FP in js/regexp/always-matches #10396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 19, 2022
Merged

JS: Fix FP in js/regexp/always-matches #10396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b4e6fb7
JS: Consider empty regexp to be obviously empty
asgerf Sep 13, 2022
f411798
JS: Fix typo in alert message
asgerf Sep 13, 2022
eca2632
JS: Add change note
asgerf Sep 13, 2022
87ab16a
JS: Update test expectations
asgerf Sep 13, 2022
d3d47a2
JS: Accept test output again
asgerf Sep 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion javascript/ql/src/RegExp/RegExpAlwaysMatches.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ predicate isUniversalRegExp(RegExpTerm term) {
or
child.(RegExpCharacterClass).isUniversalClass()
)
or
term.(RegExpSequence).getNumChild() = 0
}

/**
Expand Down Expand Up @@ -116,6 +118,6 @@ where
call instanceof RegExpSearchCall and
not term.getAChild*() instanceof RegExpDollar and
message =
"This regular expression always the matches at index 0 when used $@, as it matches the empty substring."
"This regular expression always matches at index 0 when used $@, as it matches the empty substring."
)
select term, message, call, "here"
6 changes: 6 additions & 0 deletions javascript/ql/src/change-notes/2022-09-13-regexp-always-matches-fp.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: minorAnalysis
---

- The `js/regexp/always-matches` query will no longer report an empty regular expression as always
matching, as this is often the intended behavior.
5 changes: 2 additions & 3 deletions javascript/ql/test/query-tests/RegExp/RegExpAlwaysMatches/RegExpAlwaysMatches.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,5 @@
| tst.js:22:11:22:34 | ^(?:https?:\|ftp:\|file:)? | This regular expression always matches when used in a test $@, as it can match an empty substring. | tst.js:22:10:22:43 | /^(?:ht ... test(x) | here |
| tst.js:30:11:30:20 | (foo\|bar)? | This regular expression always matches when used in a test $@, as it can match an empty substring. | tst.js:30:10:30:29 | /(foo\|bar)?/.test(x) | here |
| tst.js:34:21:34:26 | (baz)? | This regular expression always matches when used in a test $@, as it can match an empty substring. | tst.js:34:10:34:35 | /^foo\|b ... test(x) | here |
| tst.js:58:20:58:25 | [a-z]* | This regular expression always the matches at index 0 when used $@, as it matches the empty substring. | tst.js:58:10:58:27 | x.search(/[a-z]*/) | here |
| tst.js:70:20:70:26 | ^(foo)? | This regular expression always the matches at index 0 when used $@, as it matches the empty substring. | tst.js:70:10:70:28 | x.search(/^(foo)?/) | here |
| tst.js:86:22:86:21 | | This regular expression always matches when used in a test $@, as it can match an empty substring. | tst.js:86:10:86:31 | new Reg ... test(x) | here |
| tst.js:58:20:58:25 | [a-z]* | This regular expression always matches at index 0 when used $@, as it matches the empty substring. | tst.js:58:10:58:27 | x.search(/[a-z]*/) | here |
| tst.js:70:20:70:26 | ^(foo)? | This regular expression always matches at index 0 when used $@, as it matches the empty substring. | tst.js:70:10:70:28 | x.search(/^(foo)?/) | here |
6 changes: 3 additions & 3 deletions javascript/ql/test/query-tests/RegExp/RegExpAlwaysMatches/tst.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ function nonWordBoundary(x) {
}

function emptyRegex(x) {
return new RegExp("").test(x); // NOT OK
return new RegExp("").test(x); // OK
}

function parserTest(x) {
/(\w\s*:\s*[^:}]+|#){|@import[^\n]+(?:url|,)/.test(x); // OK
/^((?:a{0,2}|-)|\w\{\d,\d\})+X$/.text(x); // ok
}
/^((?:a{0,2}|-)|\w\{\d,\d\})+X$/.text(x); // ok
}