Make the analyzer comment readable - #4
Merged
Merged
Conversation
A finding said what was wrong, where, and what to do in one run-on sentence, and a mismatch put both values inside it. Comparing two long descriptions meant reading a single line to the end twice. A finding now carries an optional detail, a list of label and value pairs, and the renderer puts each on its own line. The two texts sit under each other and the difference is visible. The source rules repeated the file and the line inside the message while the header already showed both, so the actual problem arrived at the end of a line nobody finishes. They say what is wrong and nothing else now. The advice itself was written for somebody who already knew the answer. SRC_ECHO_INPUT said to pass the value through Sanitize::html, it now says why a crafted link would otherwise run script on the site. META_MISMATCH said the submission is what the directory lists, it now names both files and says which one a visitor reads when. Repeated advice is printed once per section. Two fields disagreeing used to mean the same paragraph twice, which buried the fields.
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
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.
The comment on #1 was hard to act on, so this is a pass over how a finding is presented and how it is worded.
Before
Both values are inside one sentence, so comparing them means reading the line to the end twice. The header points at
languages/en.json, but the file to change might be the submission.After
What changed
A finding can carry its values. New optional
detailon a finding, a list of label and value pairs, rendered one per line. Both filenames are named, so it is clear there are two places and either could be the one to fix.The source rules stopped repeating the location. Every one of them put the file and line inside the message while the header already showed both, so the problem itself landed at the end of the line.
plugin.phpuseseval()on line 7. →plugin.php:7— Useseval().The advice assumed you knew the answer.
SRC_ECHO_INPUTsaid to pass the value throughSanitize::html(); it now says a crafted link would otherwise run script on your site.ID_BUNDLEDsaid to choose another id; it now says every site already has that plugin and installing over it would replace part of Bludit.Repeated advice prints once per section. Two fields disagreeing used to print the same paragraph twice.
Checks
No code changed meaning, so all 63 fixtures pass unchanged — 12 corpus, 35 submission, 16 payload. Rendered both a warnings-only comment (jodit, from its real release asset) and an error-heavy one (a plugin calling
eval, an assembled function name, an unescaped$_GET, and three fields disagreeing with its zip).