perf: add fast string pre-checks to skip unnecessary parsing#456
Open
ryanquanz wants to merge 1 commit into
Open
perf: add fast string pre-checks to skip unnecessary parsing#456ryanquanz wants to merge 1 commit into
ryanquanz wants to merge 1 commit into
Conversation
Add cheap string checks before expensive RubyNode.parse and Tag.from_node calls in several linters: - NoJavascriptTagHelper: skip parsing unless source contains 'javascript_tag' - RequireInputAutocomplete: skip parsing unless source matches a form helper name pattern; skip Tag.from_node for non-input tags - RequireScriptNonce: skip parsing unless source matches a tag helper pattern; skip Tag.from_node for non-script tags - AllowedScriptType: skip Tag.from_node for non-script tags These linters previously parsed every ERB snippet or created Tag objects for every HTML tag, even when the vast majority could not possibly match.
ryanquanz
force-pushed
the
perf/fast-string-prechecks
branch
from
0b5e1df to
5b1f863
Compare
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
1 participant
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.
Summary
Add cheap string checks before expensive
RubyNode.parseandTag.from_nodecalls in several linters.Problem
Several linters parse every ERB snippet with
BetterHtml::TestHelper::RubyNode.parseor createBetterHtml::Tree::Tagobjects for every HTML tag, even when the vast majority could not possibly match. For example,NoJavascriptTagHelperparses every ERB snippet looking forjavascript_tagcalls — but in a typical codebase, >99% of snippets don't contain that string.Similarly,
AllowedScriptType,RequireScriptNonce, andRequireInputAutocompletecreate fullTagobjects for every HTML tag even though they only care about<script>or<input>tags specifically.Changes
4 files changed, +26 −1. All additive — only adds
next unlessguards before existing logic:NoJavascriptTagHelper:next unless source.include?("javascript_tag")RequireInputAutocomplete:next unless source.match?(FORM_HELPER_NAMES_PATTERN)for ERB helpers;next unless tag_name == "input"for HTML tagsRequireScriptNonce:next unless source.match?(TAG_HELPER_PATTERN)for ERB helpers;next unless tag_name == "script"for HTML tagsAllowedScriptType:next unless tag_name == "script"beforeTag.from_nodeThe tag name is read directly from the AST node (
tag_node.to_a[1].loc.source) which avoids creating the fullTagobject.Impact
The improvement scales with the ratio of non-matching to matching tags/snippets. In a typical codebase with thousands of ERB snippets and HTML tags but very few
javascript_tagcalls or<script>tags, this avoids the vast majority of parsing work in these linters.No new tests needed — existing specs cover both "offense found" (guard passes, full logic runs) and "no offense" (guard filters correctly) paths.