refactor: Restructure linters #69
Conversation
|
I'll look into the test failures on Windows later today |
| await Promise.all([ | ||
| lintXml(params), | ||
| lintJson(params), | ||
| lintHtml(params), | ||
| ]); |
There was a problem hiding this comment.
With the former implementation those were returning the transpiled source code, messages and source maps.
We might need those in the future as we would like to eventually analyze html's script content, build & check code against manifest.json directives etc.
I'm not sure I understand how this is addressed in that PR.
Currently the TypeLinter checks for js/ts files and analyzes them. However, I'm missing the mechanism that would bring the transpiled html resources, for example to that analysis there.
There was a problem hiding this comment.
Here are some examples that this functionliaty is missing with this PR:
There was a problem hiding this comment.
As an example, have a look at the XML-Linter in this PR:
https://github.com/SAP/ui5-linter/pull/69/files#diff-1bc919fd57fdbc38b20c5feaca329ba1764224846e55d4a8b7b654f18e9ca35cR29-R40
The transpiled resources are written to the workspace from where the TypeLinter will later read them together will all other ".js" resources.
The association back to the original (XML)-Resource is done by reading the original file name from the "sources" array of the associated source map in the TypeLinter's reporter: https://github.com/SAP/ui5-linter/pull/69/files#diff-1a8fcfb41f5f0e0d7d87295b6290a8af77f0d01a7881196967276fa790410ebbR148
I think the same can be applied for the HTML linter:
src/linter/html/transpiler.tsreturns one or more JavaScript + SourceMap pairssrc/linter/html/linter.tscreates@ui5/fs/Resourceinstances and writes them to the workspace- TypeLinter picks them up with all the other JS files
RandomByte
force-pushed
the
refactor-linter-modules
branch
2 times, most recently
from
12df583 to
583e026
Compare
This is our first major refactoring. Several concepts didn't play out
the way we initially thought, and that's ok.
Highlights are:
* Reporters shall no longer produce actual "reports" but rather
create "LintMessages" and "CoverageInfo"
* Linters become more independent but share a common signature
("LinterParameters")
* Linters access and share resources via a @ui5/fs
"workspace" instance. Resource paths reflect UI5 runtime paths
* Linters are generally async and executed concurrently
(eventually in workers if we can proof performance improvements)
* A LinterContext instance is proveded to all linters.
* Linters provide it with their produced "LintMessages" and
"CoverageInfo" from which the final "LintResult" is generated.
* The "ui5Types"-Linter is special and executed after all other linters
have finished (to take any transpiled resources into account)
* Use "ResourcePath" for virtual paths (= always POSIX) * Use "FilePath" for physical FS paths (POSIX or Windows)
RandomByte
force-pushed
the
refactor-linter-modules
branch
from
35b26d0 to
8996cc1
Compare
This is our first major refactoring. Several concepts didn't play out
the way we initially thought, and that's ok.
Highlights are:
create "LintMessages" and "CoverageInfo"
("LinterParameters")
"workspace" instance. Resource paths reflect UI5 runtime paths
(eventually in workers if we can proof performance improvements)
"CoverageInfo" from which the final "LintResult" is generated.
have finished (to take any transpiled resources into account)
Architecture Overview
Performance
I compared the current main (70b719a) branch with commit 881cdb2 of this branch for linting sap.m for OpenUI5 tag
1.120.10ui5lintui5lintui5lint --file-paths src/sap/m/Table.jsui5lint --file-paths src/sap/m/Table.js