leancodepl
Migrate lints to analysis server plugin
In separate PRs:
- ~re-write tests~
- move implementation to
src
What's not yet available:
use_design_system_item: separate lint rules per config entryprefer_center_over_align: our current implementation requires an asynchronous resolution of another library, which isn't (yet?) possible
(cc @shilangyu, I can't add you as a reviewer 🙈)
Actually, I might need to include the tests in this PR, since the check is complaining
What is your opinion on how the tests look like now?
To me, they seem not maintainable and not made for humans to edit. I also do not like that we are mocking libraries rather than testing on the real thing.
If you think the previous testing framework was better, I can try to write something that will make it work (if I find time). We can improve on what custom lint did and expect lints in some specific range rather than in the next line. Either way, I guess this is not mergable until we figure out how to support configs (which are crucial)? There could be a gradual migration of lints that don't need custom_lint features but still stay with custom_lint for the rest. What do you think (or, what is LeanCode's roadmap for leancode_lint here)?
What is your opinion on how the tests look like now?
Not particularly fond of them, honestly 😅
they seem not maintainable and not made for humans to edit
That's not a huge issue; the testing tooling suggests appropriate locations:
Found but did not expect:
use_design_system_item.use_design_system_item [57, 8, Scaffold is forbidden within this design system., "Use the alternative defined in the design system: LftScaffold."]
use_design_system_item.use_design_system_item [106, 4, Text is forbidden within this design system., "Use the alternative defined in the design system: LftText."]
To accept the current state, expect:
lint(57, 8),
lint(106, 4),
Either way, I guess this is not mergable until we figure out how to support configs (which are crucial)
I mean, the config, even if not done "officially", already works
That's not a huge issue; the testing tooling suggests appropriate locations:
Found but did not expect: use_design_system_item.use_design_system_item [57, 8, Scaffold is forbidden within this design system., "Use the alternative defined in the design system: LftScaffold."] use_design_system_item.use_design_system_item [106, 4, Text is forbidden within this design system., "Use the alternative defined in the design system: LftText."] To accept the current state, expect: lint(57, 8), lint(106, 4),
I don't see this being useful. 57, 106 means nothing to me, I don't know which range this is indicating
That's not a huge issue; the testing tooling suggests appropriate locations:
Found but did not expect: use_design_system_item.use_design_system_item [57, 8, Scaffold is forbidden within this design system., "Use the alternative defined in the design system: LftScaffold."] use_design_system_item.use_design_system_item [106, 4, Text is forbidden within this design system., "Use the alternative defined in the design system: LftText."] To accept the current state, expect: lint(57, 8), lint(106, 4),I don't see this being useful.
57, 106means nothing to me, I don't know which range this is indicating
Yes, in this regard the experience is very poor. I just end up selecting text in the editor and seeing how many characters I select 🙈
@shilangyu Actually, I'll try refactoring the tests to use TestCode and range-based markup (https://github.com/dart-lang/sdk/issues/61889, maybe someday it'll be oficially supported; for now it works with a src import)