Remove obsolete rule javascript.lang.correctness.no-replaceall#3998
Open
munzzyy wants to merge 1 commit into
Open
Remove obsolete rule javascript.lang.correctness.no-replaceall#3998munzzyy wants to merge 1 commit into
munzzyy wants to merge 1 commit into
Conversation
String.prototype.replaceAll is ES2021 and Baseline widely available since August 2020 (Chrome 85, Firefox 77, Safari 13.1, Node 15). The rule's warning that replaceAll 'is not supported in all versions of javascript' is no longer true, so every match in the correctness category is a false positive on valid modern code. Fixes semgrep#3984
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to an issue, if relevant
Fixes #3984
Why remove this rule
javascript.lang.correctness.no-replaceallwarns thatString.prototype.replaceAll"is not supported in all versions of javascript" and suggests rewriting toreplace()with a regex. That was fair when the rule landed in October 2020 (#887), but replaceAll is ES2021 and has been Baseline widely available since August 2020: Chrome 85, Firefox 77, Safari 13.1, Node 15 (MDN). The rule's only reference is a three.js forum thread from 2020. At this point every match is a false positive on valid code, filed under correctness.The suggested rewrite is also a small footgun. The pattern only matches string-literal first arguments, and mechanically converting something like
s.replaceAll(".", "-")tos.replace(/./g, "-")changes the meaning unless you remember to escape regex metacharacters. So the rule nudges people from the safer construct toward the riskier one.If you'd rather keep coverage for legacy-browser targets, the alternative would be a rewritten message under a compat/portability category instead of correctness. Removal seemed to match precedent like #3496, so that's what this PR does. Happy to rework it the other way if you prefer.
What I checked
semgrep test javascript/lang/correctness/passes before (5/5) and after (4/4) the removal, andjavascript/lang/passes 41/41 after, on semgrep 1.169.0.title.replaceAll(" ", "-")) and it fires as a blocking correctness finding.no-replaceall: only the rule file and its test file reference it, so nothing else needed updating.The new-rule checklist doesn't apply here since this removes a rule.