Release 3.4.0: audit fixes - #6
Merged
Merged
Conversation
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.
A review pass over the extension. Each finding below was reproduced before it was fixed and is covered by a test.
Defects that produced wrong output
VariableSymbol.columncame fromraw.indexOf(name), which finds the letter inside a keyword:var a = 1reported column 1, so the command wrotevint ar a = 1, andvarip r = 2becamevaint rip r = 2. Columns now come from the tokens. Go to Definition and Rename used the same column and were wrong by the same amount.forcounters were typedfor.for i = 0 to 10matched the variable pattern withforas the type, sox = iinside the loop was annotatedfor x = i. Counters are typedintnow,for x inandfor [index, element] indeclare their names, and all of them are scoped to the loop block instead of the rest of the file.typeblock shifted every field by one. They now carry the line they are written on.Performance
The offline checks were quadratic: every symbol rescanned the whole token stream, and shadow detection scanned every function against every variable. One index per document version, built in
sourceIndex(), makes both linear.Median of seven batches. The checks run 300 ms after every keystroke, so the old figure was felt while typing.
Completion was measured too and left alone: building the documentation for every bare built-in costs 0.09 ms, so deferring it would have been noise.
Also
//@paramtext; it previously showed nothing.CONTRIBUTING.mdcovers the modules added in 3.2.0 and 3.3.0 and says where identifier-walking code belongs.Test plan
npm test(329 tests, 7 of them new regression tests)npm run buildandvsce packagevar int a = 1,for [idx, el] in, an annotated loop body) verified against the compiler