Upstream edits and fixes from the linter. #754
Conversation
rictic
commented
Nov 15, 2017
rictic
commented
- CHANGELOG.md has been updated
src/model/warning.ts
Outdated
| * issue completely then it should go in `fix`. | ||
| */ | ||
| readonly fix: Edit|undefined; | ||
| readonly actions: ReadonlyArray<Action>|undefined; |
| * delineate. Roughly speaking, if 99% of the time the change solves the | ||
| * issue completely then it should go in `fix`. | ||
| */ | ||
| readonly fix: Edit|undefined; |
There was a problem hiding this comment.
I was thinking about this style for consistent object shape. It doesn't actually assert that we assign to the property in the constructor though so it doesn't actually matter though.
Happy to change to fix?, I guess I'm just holding a torch for microsoft/TypeScript#8476
There was a problem hiding this comment.
without an initializer the emitted code is the same, and I find ? easier to read by a hair. If you care about share you need add = undefined
src/model/warning.ts
Outdated
|
|
||
| export type Action = EditAction | { | ||
| /** To ensure that type safe code actually checks for the action kind. */ | ||
| kind: 'dummy-action-kind'; |
There was a problem hiding this comment.
Huh, interesting thought. It looks like that doesn't work well with discriminated unions:
There was a problem hiding this comment.
I actually meant 'never' :)
There was a problem hiding this comment.
ah quote escaping. the cause of and s̸̨̠̻͚̲̫̜̞̫̝̮ò̷̖̲̦̟͎͙̜̖̤̝̤̣̥͠l̴̷̡̫̲̤̪̬̙̟̀u̵̡҉͖̭̝̥̺͍̺̮͚̣͔̘͍̹͈͞͠t̢̝͔̗͙̰̝͎͚̗͎̱͞i҉̖͖̠͈͡ͅo̸̧̞̩̻͓̹̞̣̻̖̫̼̥̲̭͉͇̗̩͠͡ͅņ̷̦̞͖̮͓͍̪̳̭͙͝ ̛̥͚̥̟͉̩̯͚̳̰͉̻̦̱͖̦̮͘t̡̩̬̣̕o̷͙̥̞̰̪̪̘͚̝̬͇͢͠ ̵͏̶̪̟̦̭̙͇̹͇̼̲͟ą̕͢͏̷̪͚̰͉͖͔̭͕̝̝̥l̝̼͉͓̬̣̼̦̬͟͠l̶͍̼̹̙͈̼̭̺͈̞̞̜͇͖̀͟͜ ͏͎̩̘̱͕͖̳͍̥̼͈̖͓̜͢ͅó̞̬̙̜̬̲̝̺̬̻͖̘̗̖͉̱͍̜̕͠ͅf̴̨̧̗̲̞̱̤͔̮͍̮̻̲̼̜͈̕͞ ̴̼̰͚̯̝̪̺̳̖̗̤͈̮̹̯̲̹͘͢o̸̫͈̳͍̭̖͇̞̱̻͖̩͢u͎͈̠͓̜͇̩̯̖̬͎͚̣͓̮̰̠͇͢͞r̢̧͞͏̡̺͎̳̘͙̘̖ ̸̴̶̰̭̠̙̘͕̜̙̘̹̜̰̥̦̗͎̀ͅ7̴̧͞͠͏̪̺̬̹1̢̠͈͕̥͙̻̲͘͘͝͡ͅc҉̢̨̗̥͈̯̫͈̲̟̻͎͇̠̟̗̝̙͟ͅ3̧̕҉̜̼̱̮̩̮̞̼̭͓̻̳̫̗̪̀a̫̻̤̳̮̣̟̺͟b̸̧̻͙̙̰̙̣̤̰̳̺̼͠e̷͍͇͈̱͍̝͕͎̮͔̜͝c̸̸̨͇͍͕̠͔͇̳̝͉͓͖̖̙͠f̵̭̺̝͇͟͞g̨͈͖̮͇͔̥̱̤͔̪̺̮̦͟͠͝b̶̛̪̣̯̝͉̣̠͈͖̞͎̼͖͎͍̯͎͘͢͠1̷̶̱͈̖̲̜̤͎̠͕̮̹̪͕̥͉̠͟͠c̢͜҉̧̠̗͉̦̪ȩ̱̯̹̭̪̣̪͚͇͓͡c҉͍̦̫͈̞͉͓̬͞g̷̝̪̘̼͇͈͍̘̩̝͇̝̳͓͙͜3̶̡̢͖͙̲͇͔̝̥̞̕f̸̲̞̣̤̟̝̦̝͓̟͈̩͍̼͍̟͡c̷̴̨͙̖͙͓̩̦̦̯͕͙͎̥͈͙̮̦̀1̵̹̭̠̥͉̠̣̘̻̠b̢̙͖̹̘͍̭̪͉̪̙͍̮f͏̺̣̺̣͚͔̱̀2̶̸̷̢͕͕̗̲̦̤̺̠̞͉̟̭͇̭̘̜̭̤͕5͕͇̱͍͖̺́͠b̶̶̢͉̼͔̘̗̰̮̰̭͞͞6̡̙̳̼̥́4̩̭̥̺̠̣͓͔̠̺̟̞̻̥̲͚̩̖̕͟1̖̥̙͓̣̱̤̝͕̯̥̬̣̙̼̦͉̱͈̀͢͟6̷͘҉͓̪̭͇̖̮̰̺͚̰̳̦͕͙̝̙͜0̛̭̼̗̮̣̣̟̝̘̣̲̯͎̦̙̰ͅ9̡͏̧̛̣̗̮͙̲̥̗̩̰̘͕̫̪̠̭̼̯2҉̢̢͙͙̝͓̭̺̀ͅ4̨͓̗͓̗̳̗̲͚̰̞̝͇̯̠͞͠8̵͔̠̘̹͖̱̬̭͚͈͇̫͓̤̼͜
| * If there are overlapping edits, then edits earlier in the array get priority | ||
| * over later ones. | ||
| */ | ||
| export async function applyEdits( |
There was a problem hiding this comment.
why does the analyzer have code to apply edits?
There was a problem hiding this comment.
It's useful for multiple downstream projects. The CLI and the IDE.
I suppose both depend on the linter as well as the analyzer, but I feel like the applyEdits function is closely tied to the meaning of an Edit as an atomic unit of change.
Also the only legible way to test edits is to apply them and assert on the result. We'll probably want to add some fixes and edits in the core analyzer scanners, and we'll want to test them too.
| * delineate. Roughly speaking, if 99% of the time the change solves the | ||
| * issue completely then it should go in `fix`. | ||
| */ | ||
| readonly fix: Edit|undefined; |
There was a problem hiding this comment.
without an initializer the emitted code is the same, and I find ? easier to read by a hair. If you care about share you need add = undefined
| }); | ||
| for (const replacement of replacements) { | ||
| const offsets = document.sourceRangeToOffsets(replacement.range); | ||
| contents = contents.slice(0, offsets[0]) + replacement.replacementText + |
There was a problem hiding this comment.
soo bad we don't have String#splice()
There was a problem hiding this comment.
Yeah, this was surprisingly awkward.
src/model/warning.ts
Outdated
| replacements.set(replacement.range.file, [replacement]); | ||
| } else { | ||
| const fileReplacements = replacements.get(replacement.range.file)!; | ||
| // TODO(rictic): insert in sorted order |
There was a problem hiding this comment.
remove TODO? the later sort seems fine
rictic
force-pushed
the
edits-and-fixes
branch
from
cbdf919 to
90662a3
Compare
