Integrate with ESLint

[RandomByte]

Requirement

Provide an ESLint plugin to easily integrate UI5 linter checks into existing development workflows.

Challenges

Or why this was not our initial design goal

• ESLint rules only check one file at a time. There's no built-in way of holding a context of the project, so that imported modules from the same project can be found and used to gather type information
  • A key requirement of UI5 linter is the ability to resolve indirect usage of deprecated API. This is done by resolving the (TypeScript-) types of dependencies used in a module and checking every access to them for usage of deprecated API
  • This is very similar to what for example typescript-eslint calls "type-aware linting"
  • ESLint might provide functionality for "project aware linting" in the future, see also: Feedback Wanted: Project-aware ESLint eslint/eslint#16819
• ESLint creates an AST representation of every source file. In order to lint non-JS resources such as XML-views, the UI5 manifest.json or ui5.yaml, we would need to provide a set of custom processors or even a custom parser
  • See for example the eslint-plugin kennyzhangdev/ui5depchk which makes use of a custom parser for XML resources
• Unclear integration with typescript-eslint: UI5 JavaScript files first need to be transpiled (from our proprietary AMD implementation to ESM), before doing the type-aware analysis using the TypeScript compiler. The latter could potentially be realized using a plugin for typescript-eslint (similar to for example gund/eslint-plugin-deprecation, but it's unclear how the first step can be integrated in that process
  • Additionally, since we are explicitly targeting projects that are not using Typescript, we would need to find a way to configure typescript-eslint for our own plugin. For example we need to provide it with our predefined tsconfig (the project typically doesn't have one), and we need to inject the UI5 type definitions provided by UI5 linter
    • Otherwise every project that wants to use UI5 linter would first need to properly setup TypeScript, even though it is not used for anything but the linter
  • UI5 linter currently declares its own dependency to typescript (required for linting and transpilation), while typescript-eslint requires projects to define the dependency themselves
  • typescript-eslint itself shows the limits of integrating "type-aware" linting into ESLint, especially with regards to performance (see https://typescript-eslint.io/packages/parser#allowautomaticsingleruninference, Performance Opportunities typescript-eslint/typescript-eslint#6366)
• ESLint requires synchronous implementations of custom processors and parsers (see Is there a plan to support asynchronous custom parser api? eslint/eslint#14733 and Change Request: allow asynchronous parser eslint/eslint#15475)
  • Some of the UI5 Tooling API currently used within UI5 linter is asynchronous. Providing synchronous alternatives would require substantial development efforts

Proposal

• Monitor development on ESLint and typescript-eslint side
• Focus on providing a good out-of-box experience
  • Installing the UI5 linter CLI and execute ui5lint should just work for the majority of today's UI5 projects. Projects should typically not need to be modified
  • By requiring users to install additional dependencies and to maintain (eslint) configuration, we expect a much lower level of adoption, making such scenarios a secondary requirement.
• Provide Node.js API for individual linting steps to enable integration in third-party projects
  • This might include ESLint plugins provided by the community
  • Other possible consumers are existing linting solutions such as https://github.com/kennyzhangdev/ui5depchk, https://github.com/iljapostnovs/ui5plugin-linter or https://github.com/SAP/ui5-language-assistant
• Investigate LSP implementation to provide direct IDE integration

[mauriciolauffer]

Challenges

• Pretty much all the problems listed here were solved by other communities. Using a custom parser for specific file formats is not a problem. React, Vue, Ember and others have been using it for years. At the end of the day, this new tool is another custom parser, right?
• Dismissing ESLint with arguments like would require substantial development efforts is a bit weird to justify a new tool built from scratch;
• Today, the tool doesn't offer anything that couldn't be achieved with ESLint, on the contrary, it offers fewer features;
• Linting is much more than checking deprecated APIs, see the aforementioned plugins, they do it and much more;
• Will this tool offer rules for security, code complexity, styling and other advanced use cases? For instance, eslint-plugin-security;
• ESLint is already used in multiple UI5 projects by customers, partners and SAP itself. They use a plethora of ESLint plugins including for styling (such as Airbnb). Now, they have to use 2 linting tools... Great!
• The same will happen in CAP and Web Components projects. Developers will need to use ESLint for them and another tool for the UI5 bit. No reusable rules or styles. Double great!

Proposal

• ESLint (js/ts) has tools and communities more than mature. What this tool will do that they don't?;
• Installing ESlint works with all UI5 projects today, no need to modify them, a config file is all one needs;
• Anybody who knows what linting is, uses ESLint today. The only ones NOT using ESLint are those who won't use this new tool as well because they have no idea what's it for or don't care. SAP is setting a lowest common denominator community;
• Why complicate things? This is using ESLint with extra steps and doesn't make any sense;

PS: I'm sorry if this sounds too harsh, but I've spoken to multiple SAP developers working on real projects, and nobody could understand why or see the value in this new linting tool...

[codeworrior]

I think this is a big misunderstanding. For sure, ESLint is the 1st choice for linting JavaScript / TypeScript and its other derivates. The UI5 linter doesn't want to replace ESLint. It therefore won't get rules that exist already in ESLint or better could be added there (the linter's repository might in fact become the home for such rules / for a UI5 specific ESLint plugin, should we decide to implement it)..

And yes, the UI5 linter contains several custom parsers.

But that's not the point. There's one big difference to ESLint rules and @RandomByte already explained it above. To reliably find usages of deprecated APIs, it's not sufficient to scan a local AST as ESLint rules do. You have to build up a project-wide understanding of types to judge whether a getProperty call is a ManagedObject.prototype.getProperty call (not deprecated) or a v4.ODataModel.property.getProperty call (deprecated). This is something you don't get from ESLint yet (please read the ESLint issue #16819 linked above).

For sure, we could find ways to bypass this, creating a project view on our own and accessing it from within our rules. But that's a substantial development effort (I'll come to this in a second). And there's no guarantee that such an approach then will survive a major version update of ESLint (multi threaded execution of rules which works well with isolated AST-visiting rules, but obviously adds additional effort for shared project information). If ESLint doesn't provide a way to maintain a project view, we're at risk that our workaround doesn't work anymore.

Reg. the effort: one of the major requirements for the UI5 linter was a quick availability. That's why substantial development efforts was not so weird as a counter argument from our point of view. We therefore decided not to re-invent the wheel and rather integrate with the TypeScript compiler.

You might argue that we at least could integrate all those checks that can be implemented on a per-file base as ESLint rules. That's true, but at least currently we think it's more beneficial for UI5 developers if a single tool helps them to identify UI5 specific potential for improvement in their code.

Last but not least, if we later find out that our requirements regarding the code scans are fufilled by a newer ESLint version, we will check if / how we can integrate into it.

Again, UI5 linter isn't there to replace ESLint, it's rather an addition (additional tool). Maybe, from that point of view, the name choice was a bit misleading (it's rather a UI5 deprecation and best practice checker). But we thought that the term "linter" is so familiar to developers that it's the best choice to explain the purpose of the tool.