Creating Lint Rule logic: seeking tips-and-tricks

[Patt O'Brien]

TLDR: Say you had to onboard a new team member of the Linter package: what tips and tricks would you share to help them get up to speed with writing lint rules most efficiently ?

I'm currently building a dev tool based on Analyzer, that use custom lint rules on Dart/Flutter code. I've been primarily working with the analyzer for over a year, have read all of the docs on writing custom rules in the analyzer_plugin, analyzer, and linter packages, and am mostly familiar with how the tool works in general.

When it comes to writing lint rules, I'm having a bit of trouble building a good mental model for creating AstNode-specific logic. Unlike other aspects of software development where I can “forsee” the logic before writing any code, for lint rules I build up coverage for rule use cases via an ad hoc TDD workflow, for example:

• set up an e2e test using a source code file with a couple different examples
• run the test using the debugger to view runtime properties of AstNodes, which can help guide AstNode-type-specific rule logic
• think up more test examples that can break the existing logic; rinse and repeat

The above workflow has actually been working relatively well for me, and the new sealed class modifiers + exhaustiveness checking features have further helped me wrap my head around the inheritance model of certain AstNodes. But it all still feels a little too trial and error than what I’m used to, and I don't ever feel 100% certain that my rules have captured all edge cases.

As a concrete example, see the ARB Resource value parser attached. I’ve written several of these “Expression-agnostic” functions that use switch expressions to capture specific values of every type of Expression. While I do eventually find the commonalities of all of the different nodes in order to refactor a “more exhaustive” implementation, it's not that intuitive (yet, I suppose).

(see attached example file)

Anyways, some specific questions I've thought of:

• Do certain patterns exist that are often used ? e.g. recursive expression evaluation, capturing variable declarations that match a given predicate
• Is it possible to know with certainty if you've captured all use cases? or is it all trial-and-error?
• Is there another workflow that you'd recommend besides the aforementioned TDD approach?
• is there a Discord server or similar forum for discussing analyzer tips and tricks with folks working on similar projects? (I'd be interested in starting something if others would like to join in)

Thanks in advance for any insight! It's greatly appreciated :)

[Brian Wilkerson]

I was holding off responding in the hope that other community members would chime in because I think their insights would be helpful, but I also don't want you to think that we're ignoring you.

I'm having a bit of trouble building a good mental model for creating AstNode-specific logic.

Unfortunately, that's not too surprising. The language is fairly large, so the AST representation is also fairly large. And there's support in it for experimental features, some of which is incomplete.

If you can think of anything, I'd be very curious to know what kind of information / tooling would help with this problem. Should we improve the documentation on the classes, add something in the `docs` directory that acts as a roadmap, or is there an even better way for us to help?

One tool we have that I didn't see mentioned is in the Analysis Server Diagnostics pages. (In VSCode you can open them from the command palette, in IntelliJ there's a button in the Dart Analysis view.) If you click on "Contexts" in the left column, then scroll down to the lists of "Context files", to the right of every file path there's an "ast" link. Clicking that link will open a tree view of the AST for the entire file. That might be an easier way to explore the structure than the debugger.

... I don't ever feel 100% certain that my rules have captured all edge cases.

Also not surprising. The original design of the AST was suboptimal in some respects, and some of that still remains. And the language has changed a lot since we first wrote the AST classes (they've been around since before Dart 1.0), but it's always been deemed too expensive to completely redesign and rewrite them, so they've grown somewhat organically (though hopefully with more consistency than "organically" might imply). We do keep making improvements, but it's a slow process.

It's also made harder by the fact that the AST structure after parsing a file can be different than it is after resolving the file. If you can afford the extra time (in terms of performance) I'd strongly advise that you always use a resolved AST. If you have to live in both worlds it will be much more confusing.

Do certain patterns exist that are often used ? e.g. recursive expression evaluation, capturing variable declarations that match a given predicate

The lints in the linter package make use of an `AstVisitor` to handle recursively traversing the structure. They do this when looking for nodes that need to be analyzed as well as for finding nodes or elements matching some criteria (for example, references to a local variable). They rarely do anything similar to your example, where you're evaluating code, and I wouldn't use an `AstVisitor` for that purpose, primarily because the visit methods don't handle lists of values very well.

Is it possible to know with certainty if you've captured all use cases?

Yes (modulo human error), but it requires a fairly complete understanding of the language, the structure of the AST, and the requirements of the analysis you're trying to write.

For example, if you're trying to look at all invocations of a constructor, you need to look at instance creation expressions, but you also need to look at all the nodes representing the way constructors can be invoked directly from another constructor (such as a super constructor invocation in the initializer list). If you're looking for references to a method and if "references" includes tear-offs, then you need to look at both method invocations and property accesses.

Is there another workflow that you'd recommend besides the aforementioned TDD approach?

I like TDD, but even with that it helps if you are familiar with the language so that you can write the tests.

is there a Discord server or similar forum for discussing analyzer tips and tricks with folks working on similar projects?

I'm not aware of one. The Flutter team has embraced Discord, but the Dart team, which includes the analyzer team, largely hasn't. It's unfortunate, but I, for one, probably wouldn't see your questions on Discord unless you mentioned me explicitly. Then again, you might get a better response from the larger community there than you have here. I'm not sure there's a good answer here.

--
You received this message because you are subscribed to the Google Groups "Dart Analyzer Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to analyzer-discu...@dartlang.org.
To view this discussion on the web visit https://groups.google.com/a/dartlang.org/d/msgid/analyzer-discuss/efc3ea32-eda2-4c36-b1bf-39ba484954ccn%40dartlang.org.

[valauska...@gmail.com]

I tried to "superimpose" the implementation of various visitors onto the AstNode hierarchy:

- here is a directory that contains some of that work: https://github.com/modulovalue/graphs/tree/main/%2306_2023-04-03_visitors

- and here is a graph that superimposes a code example for most AstNodes onto the AstNode hierarchy: https://github.com/modulovalue/graphs/blob/main/%2301_2023-04-03_astnode_with_examples.pdf

Note that the hierarchy there is out of date, it does not contain recent changes to the AST.

This was very useful to me, maybe somebody will find some value in it, too.

There is an unofficial Dart Discord server here: https://discord.gg/GraRQEBC and a hackers-dart channel on the official Flutter Discord server.

[Dmitry Zhifarsky]

> I was holding off responding in the hope that other community members would chime in because I think their insights would be helpful

And I was waiting for your response 😅.

Adding my 5 cents:

> Do certain patterns exist that are often used ? e.g. recursive expression evaluation, capturing variable declarations that match a given predicate

For me it's mostly "check that the DartType is X" where X is Column, Row or any other class, removing '_' from a name and getting a named argument by its name. But other than that rules rarely reuse any logic and a pattern that works for one rule can be bad for another one.

> Is it possible to know with certainty if you've captured all use cases? or is it all trial-and-error?

Trail and error for sure. What helps is having a list of popular repos (ex. 10-20) on which you can run your rules. It's preferable that those repos have different code style. Plus the language is always evolving and the analyzer itself being actively worked on. For example, soon we'll get extension types and augmentation, so even if you thought that you've captured all use cases with a new release that can be not true anymore.

And I disagree with Brian a bit, considering that the analyzer and the linter sometimes also have these problems of not covering some cases. So even with the experience it's still possible to make this mistake.

> Is there another workflow that you'd recommend besides the aforementioned TDD approach?

IMO, that's the best workflow so far. It helps you explore and learn the AST while developing new rules, your code is always covered, meaning when the analyzer introduces a breaking change you are sure that your code works or you see what is broken and you get a nice bonus when people show you false-positives, you just copy their code into your tests and can identify the problem really fast.

> is there a Discord server or similar forum for discussing analyzer tips and tricks with folks working on similar projects?

If there is no option yet, I'd love to host a dedicated channel for that on the DCM Discord.

To Brian:

> but it's always been deemed too expensive to completely redesign and rewrite them

Do you have any design documents on how the AST can be improved? I'd love to take a look on that.

> If you can think of anything, I'd be very curious to know what kind of information / tooling would help with this problem. Should we improve the documentation on the classes, add something in the `docs` directory that acts as a roadmap, or is there an even better way for us to help?

IMO, the vast majority of problems come from not really knowing all possible "parent -> child" relationships. And `AstNode? get parent;` is the main contributor to this confusion. If we could have an option of union types or at least of comments describing possible relationships, that would help a lot. Basically what @modulovalue has shown, but closer to the source code. But I do understand that supporting this documentation might be not worth it.

[Brian Wilkerson]

And I disagree with Brian a bit, considering that the analyzer and the linter sometimes also have these problems of not covering some cases. So even with the experience it's still possible to make this mistake.

That's completely fair. We're all human and we all make mistakes. The important point I was trying to make is just that the more familiarity you have with the language the easier it is to think of all the corner cases that need to be covered. I'm certainly not perfect, and I'm often amazed by some of the cases that the folks on the language team think of.

Do you have any design documents on how the AST can be improved? I'd love to take a look on that.

No we don't. That's something I've been meaning to get to (for years now), but there's always something more pressing that needs my attention. :-/

IMO, the vast majority of problems come from not really knowing all possible "parent -> child" relationships. And `AstNode? get parent;` is the main contributor to this confusion. If we could have an option of union types or at least of comments describing possible relationships, that would help a lot. Basically what @modulovalue has shown, but closer to the source code. But I do understand that supporting this documentation might be not worth it.

I can't add union types, that would have to be a language enhancement. But I can certainly think about adding documentation. Whether or not we actually enhance the documentation, the concrete suggestion is much appreciated.

[Dmitry Zhifarsky]

> That's completely fair. We're all human and we all make mistakes. The important point I was trying to make is just that the more familiarity you have with the language the easier it is to think of all the corner cases that need to be covered. I'm certainly not perfect, and I'm often amazed by some of the cases that the folks on the language team think of.

I completely agree with you, I also make these mistakes and with each DCM release add a whole bunch of fixes just because of that. And can confirm that with time it becomes easier, but still hits from time-to-time 🥲.

> No we don't. That's something I've been meaning to get to (for years now), but there's always something more pressing that needs my attention. :-/

I just hope you guys get some time from all the new changes and address all the "we actually need to rework this" things at some point. Not sure that any external contributor can help with that. Even though I'm trying different ideas on how the analyzer can do things differently, I'm only looking at it from the linting standpoint and more than sure that my ideas can be a completely dead-end for all other features it has.

> I can't add union types, that would have to be a language enhancement.

I understand that, it's just an obvious (to me) option for the "gets updated with the code and is quite expressive on its own".

> But I can certainly think about adding documentation. Whether or not we actually enhance the documentation, the concrete suggestion is much appreciated.

What do you mean by a "concrete suggestion" in this context? Do you need a list of nodes for which this documentation can be useful or something else?

[Brian Wilkerson]

What do you mean by a "concrete suggestion" in this context? Do you need a list of nodes for which this documentation can be useful or something else?

No, I meant the suggestion to better document `getParent`. That's a lot more actionable than "add documentation everywhere".

[Dmitry Zhifarsky]

Looks like this might be an example of such a comment https://github.com/dart-lang/sdk/blob/main/pkg/analyzer/lib/src/dart/ast/ast.dart#L2135.

But unfortunately, it has the exact problem discussed above: it's outdated and thus misleading.

But it would be even better to see this documentation on a "node.parent" invocation for each node (which is problematic, considering the inheritance model).

For example, when called on a MethodDeclaration, it would show a comment that method declarations can be a child of a class, mixin, etc.