Support custom lint rules

[trentgrover-wf]

I'd like to be able to specify custom lint rules outside of this package. This is mostly to help enforce consistency in consumption patterns of some of our private libraries that don't apply well to the larger Dart community, but would be very beneficial to our developers.

To give 1 specific example, we've implemented a Disposable interface / mixin to assist with cleaning up streams and other data structures that won't necessarily be garbage collected without some manual intervention (https://github.com/Workiva/w_common). I'd like to be able to perform some linting similar to the existing cancel_subscriptions and close_sinks rules to verify that any Disposable object instantiated is actually disposed.

[pq]

cc @bwilkerson re: plugin support.

[devkabiir]

+1 👍 💯 Another example from me, I want to impose myself and other devs involved in the project to always store DateTimes as MillisecondsSinceEpoch (int) and only use these DateTime Constructors : DateTime.utc(), DateTime.now() and DateTime.MillisecondsSinceEpoch. Mainly to maintain DateTime consistency throughout code.

[stt106]

I think this would be an awesome feature to have! In some way, the customised linter can act like a second compiler which helps to detect more errors/code smell at compile time, leading to fewer runtime exceptions. Generally, OO compilers are less helpful in making illegal state impossible at compile, compared with FP compilers, mostly due to anything can be null. But with a customised linter, this would be improved a lot, having the benefits of both OO and FP worlds.

[stepancar]

@pq Do you have any vision how to do plugins support? Why linter is a part of sdk now? I can start research of this, but maybe you can share any information about discuss on this issue

[pq]

Sorry for the slow reply @stepancar! The original intention of the linter was for it to function as a plugin and be very loosely coupled w/ the SDK. For a variety of reasons, largely performance-related, we had to step back from that. (@bwilkerson can provide some more context.)

Anyway, I agree it would still be great to support some kind of model for pluggable analyses even if, for performance reasons, we need to keep the core linter embedded the way it is today.

Out of curiosity, what kinds of use cases are you envisioning?

[georgelesica-wf]

We wrote an internal linter using the unsupported analyzer APIs that does the following right now:

• enforce Flux events having a common base class that provides Disposable helpers
• enforce Flux actions having a common base class (same reason)
• require that Flux events and several other kinds of objects be "managed" (again, a Disposable thing)
• we implemented our own @NoImplement annotation to prevent certain classes from being used as interfaces because doing so would make semver compliance more difficult
• we implemented a @Sealed annotation to prevent overrides in certain cases

We've actually had some trouble porting this tool to Dart 2 because the analyzer has changed so much. At the time we wrote it, we met with @kevmoo and he indicated that a pluggable analyzer was coming. I know that became a thing for Flutter, but it isn't considered generally available and the documentation is very sparse, so it still doesn't help us (yet?).

I don't think we're picky about how we satisfy these use-cases, we just need to be able to satisfy them in a reasonably stable manner.

[pq]

Thanks for the added context @georgelesica-wf!

When does your tool get run? (And how regularly?)

Are you building on https://github.com/Workiva/dart_dev ?

FYI @bwilkerson

[trentgrover-wf]

It's run by a global command: pub global run dart_medic

We can run that locally at will, but we also add it to our dart_dev task runner command and run it in CI.

The types of things we check for are things that we'd love to have available in real-time in our IDE while developing, but we're making do for now, hoping we could get that sort of functionality for free from analyzer plugin support :)

[georgelesica-wf]

To expand on your last question @pq it doesn't build on top of dart_dev directly. It actually does use our semver auditing tool as a library to get the compiled elements. Unfortunately, that's the part that has proven difficult to update to Dart 2.

[bwilkerson]

We've actually had some trouble porting this tool to Dart 2 because the analyzer has changed so much.

If there are specific questions about the analyzer APIs and how to update them, I'm more than happy to answer them for you. (The API is continuing to evolve to better represent the Dart 2.0 semantics and to allow for performance improvements in the implementation, and I might also be able to help you future-proof your code.) I'm happy to use any forum you want, but I'll see github issues and e-mail without prompting; for other forums you might need to let me know that there's something to respond to.

At the time we wrote it, we met with @kevmoo and he indicated that a pluggable analyzer was coming. I know that became a thing for Flutter, but it isn't considered generally available and the documentation is very sparse, so it still doesn't help us (yet?).

Yes, we do have a plugin story for the analyzer now. It isn't Flutter specific, and Flutter isn't currently using it. It is currently being used to support Angular.

It's currently only being used by the analysis server, but we have (unscheduled) plans to add support for it to the command-line analyzer as well at some point.

We haven't made it generally available (encourages people to use it) because

• it isn't yet supported in the command-line analyzer,
• it's fairly heavyweight to use it (especially for something like a small number of fairly simple lint rules), and
• we haven't yet evaluated the performance characteristics of having large numbers of plugins running, so we don't know whether encouraging its use would hurt the UX.

I don't know when we might be able to address those concerns.

[cah4a]

Any updates?

Custom rules could improve code consistency on a particular project. Any well-coded project should have project-related conventions. The more convention checks are described as lint rules, the easier it's to follow these conventions. It will save much time on a code review stage of each PR.

I think it's a must-have feature for any linter.

Also, it will allow to make a platform or framework specific rules. Flutter, Aqueduct or DartAngular rules could be very helpful.

[pq]

@cah4a: no significant updates. I'd still love to see this happen (for many of the reasons you describe and a few more). Our current energy has been focussed on a scramble to keep up with (and support) evolving Dart language features but continued conversation here (and upvotes) will help motivate prioritization down the road, so thanks for that!

Regarding convention checks, is it possible that some of your desired ones would be generally useful? Are they worth considering implementing in the linter proper?

As for Flutter rules, could you enumerate ones you have in mind? (Folks are actively thinking about those so your ideas will be especially welcome -- #142 is an old tracking issue too fwiw.)

Finally, short of tight integration of custom lints into the IDE could you see value in a way to run custom lints from a separate (command-line) tool? A flow like @georgelesica-wf describes?

[jodinathan]

Look at the scramble I guess overloaded functions is not in the near sight

Atenciosamente,

Jonathan Rezende

Em 6 de abr de 2019, à(s) 10:11, Phil Quitslund notifications@github.com escreveu:

@cah4a https://github.com/cah4a: no significant updates. I'd still love to see this happen (for many of the reasons you describe and a few more). Our current energy has been focussed on a scramble to keep up with (and support) evolving Dart language features https://github.com/dart-lang/language/projects/1 but continued conversation here (and upvotes) will help motivate prioritization down the road, so thanks for that!

Regarding convention checks, is it possible that some of your desired ones would be generally useful? Are they worth considering implementing in the linter proper?

As for Flutter rules, could you enumerate ones you have in mind? (Folks are actively thinking about those so your ideas will be especially welcome -- #142 https://github.com/dart-lang/linter/issues/142 is an old tracking issue too fwiw.)

Finally, short of tight integration of custom lints into the IDE could you see value in a way to run custom lints from a separate (command-line) tool? A flow like @georgelesica-wf https://github.com/georgelesica-wf describes?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/dart-lang/linter/issues/697#issuecomment-480503051, or mute the thread https://github.com/notifications/unsubscribe-auth/AF6Y3s3HZCjQ-c0N9ci0_yY7CBAzju_eks5veJ0fgaJpZM4N0rfT.

[cah4a]

@pq At the moment, I want to implement some "Separation Of Concerns" checks. Eg:

• No widgets outside "/widgets" folder
• No http requests outside "/api" folder

So it's not about what code does, but where it lives. Don't think it could be generally useful.

Of course, I want to have custom lint rules because of tight IDE integration. So you could see those issues while you coding. Otherwise, you try to satisfy CI after a commit is ready. That could be annoying. If it's impossible, I will have to write my custom command with those checks as a workaround.

As for flutter rules, I think the next could be useful:

• Provide child for AnimatedBuilder
• Redundant Column, Row, Stack
• Redundant StatefulWidget
• Dispose not called for ScrollController, AnimationController, GestureRecognizer, etc
• Don't call Navigation.push/pop in build
• Use EdgeInsetsDirectional/AlignmentDirectional/Positioned.directional for non-symmetric layouting

[pq]

Thanks for the added context @cah4a. Really helpful!

Adding @Hixie and @a14n as an FYI on flutter rule ideas.

[Hixie]

See also https://github.com/flutter/flutter/issues/1220

[Levi-Lesches]

Any progress on this? This seems like something that many users can benefit from.

[pq]

Not much unfortunately. Maybe once we're through extension methods and NNBD? 🤞 (That's the team-wide priority for the immediate term.)

[baiyingmh]

Is there any progress on this, we need custom dart lint rules too, it is not universal for everyone, but it is a very meaningful constraint for our team's project

[bwilkerson]

No, there hasn't been any progress on this. While extensions have shipped, NNBD is still very much our primary focus right now.

But more generally, I'm honestly not sure what support for custom lints would look like. Every time I think about what would be required in order to support custom lints I come to the same conclusion: it would look exactly like what we already support with analyzer plugins. Unless we could do something that would make it easier for users (and that can't be applied to the analyzer plugin support to also make it easier) I don't think we're likely to do anything different.

I generally discourage people from using our plugin support because we have not finished evaluating the impact that plugins have on the performance of the analysis server, nor are plugins used by the command-line analyzer.

The alternative is to write your own linter-like support using the analyzer package. It won't give you the IDE integration that analyzer plugins were designed to provide, but you could, at least, run them from the command-line and as part of your CI system.

[pq]

Thanks Brian!

The alternative is to write your own linter-like support using the analyzer package

If you do want to explore this route, you might find this package useful:

https://github.com/pq/surveyor

Adding custom analyses is what this is all about and it would be easy to create a script to run in a CI.

Feel free to reach out if you have any questions.

[winterDroid]

Yeah I think it would be great to write custom rules as it is possible Android Lint.

I had rules like the following in my mind:

• Use Class X instead of Class Y
• Don't use Class X
• Classes extending X should be named like ...
• Don't use Colors directly. Instead use MyColors

Essentially very project setup specific rules that should help new team members to get onboarded and follow team internal rules.

[LuisReyes98]

Are there any relevant updates in this feature?

[pq]

Hey @LuisReyes98. No significant updates. Sorry!

EDIT: do you have any additional use cases to add to the list above?

[MarcelGarus]

I implemented a code-generator for has_is_getters and having lint rules that tell users to use someObject.hasProporty instead of someObject.property != null or someObject.isEnumValue instead of someObject.someEnum == SomeEnum.value would be really cool.

[LuisReyes98]

Custom rules can also help keep order in big teams of programmers, giving errors when a class doesnt have comments for documentation for example

[mateusfccp]

I'm making a sumtypes package and it would be nice to make linter be able to extensively check for verification of subtypes like it already does with enum on switch.

Example:

@Sumtype
abstract class Rank { }

class Spades extends Rank { }
class Hearts extends Rank { }
class Clubs extends Rank { }
class Diamonds extends Rank { }

// ...
void rankSymbol(Rank rank) {
    final type = rank.runtimeType;

    // Here the linter would warn that Diamonds has not been checked
    switch (type) {
        case Spades:
            return '♠';
        case Hearts:
            return '♥';
        case Clubs:
            return '♣';
    }
}

So +1 to this idea.

[bwilkerson]

... giving errors when a class doesnt have comments for documentation for example ...

This is an example of a generally useful lint that would be useful for lots of projects. (See https://dart-lang.github.io/linter/lints/public_member_api_docs.html).

[Levi-Lesches]

...check for verification of subtypes like it already does with enum on switch.

My workaround for this is to give Rank an enum field. Like this:

enum Suit {spades, hearts, clubs, diamonds}

abstract class Rank {
  Suit suit;
  Rank(this.suit);
}

class Spades extends Rank {
  Spades() : Rank(Suit.spades);
}
// other classes here

String getSuiteName(Rank rank) {
  switch(rank.suit) {
    case Suit.spades: return "spades";
    case Suit.hearts: return "hearts";
    case Suit.clubs: return "clubs";
    // warns that diamonds has not been checked
  }
}

The two biggest problems with this IMO are

1. Each subclass must now use the super constructor with a static value. More boilerplate
2. The linter warns that getSuiteName might return null, even though you clearly want each Rank subclass to have a non-null suit. But with nnbd, I think that actually gives this workaround an advantage...

[mateusfccp]

@Levi-Lesches Yeah, the problem is when the sumtypes carry information. The example I gave was the most trivial possible, but there are cases where some of the values of the sumtype has "sub values".

Eg. type List T = Nil | Cons T (List T)

In these cases, I would do:

abstract class List<T> { }
class Nil<T> extends List<T> { }
class Cons<T> extends List<T> {
    final T value;
    final List<T> tail;
}

But I don't think if it is viable to use the enum approach in these cases.

[hpoul]

@mateusfccp not really related to this issue, but you might want to take a look at how freezed implements it: https://pub.dev/packages/freezed#when .. i assume you already generate code, so you can workaround it by creating your own switch function with named arguments for each case.. with @required annotation this will trigger a warning if not passed along.

[mateusfccp]

@hpoul Yeah, I already do it... I generate a fold function that takes 1 function for each of the sumtype variants... By doing this, I can force the programmer to provide each case...

The problem is to provide an else like case... I can make another parameter in fold providing the else case, but if I do this, then I will have to make all the other parameters optional. However, by doing this:

• If else is also optional, I can't know when the user has exhaustively checked for all the variants OR provided the else case; or
• If else is a mandatory parameter, the user will have to provide an else case even if they checked for all the variants.

Neither cases are good, but I think the second one is still better, and I think I'm going this way by now...

[MarcelGarus]

@mateusfccp That's why freezed has two different methods for that:

• when, which requires every variant to be specified and
• maybeWhen, which doesn't require all variants, but requires the else parameter.

Btw, I believe https://github.com/dart-lang/language/issues/349 might be interesting for you.

But IMO this discussion is getting out-of-scope for this issue.

[ykmnkmi]

Anyone can write plugin for analyzer with analyzer_plugin. For example angular_analyzer_plugin.

[nosmirck]

I'd like to contribute with another idea, since Dart doesn't support interfaces (but implied interfaces) that doesn't stop anyone from extending a class (or abstract class) intended as an interface only.

If we had a linter that checks for an annotated class as @interface then the linter can take care of the rest and we can give whatever level of severity, either just a warning or make it a full blown error.

@interface
abstract class MyInterface extends OtherClass{  //Lint! interface must start with I, Lint! Interface cannot extend other classes
  int x; //Lint! Interface cannot declare instance fields
  void bar(){}; //Lint! interface cannot implement methods
}

//This interface is properly declared and can be used as expected
@interface
class IMyInterface{ 
  int get x;
  void bar();
}

class Foo extends IMyInterface{} //Lint! cannot extend an interface

//This class properly implements the interface
class Bar implements IMyInterface{
  int get x => 0;
  void bar(){
    print('hello');
  }
}

I might be missing more lints that we usually would have by using interfaces, but these are like the top most used features. This helps prevent developers of a library using a class that was intended as an interface to be extended.

[MarcelGarus]

@nosmirck Just a side note, not directly related to this issue: You can give classes private constructors to prevent them from being extended or instantiated in other files:

abstract class MyInterface {
  MyInterface._();
  ...
}

[chenenyu]

Looking forward to this feature! 😭

[leecommamichael]

@bwilkerson

No, there hasn't been any progress on this. While extensions have shipped, NNBD is still very much our primary focus right now.

Thank you for your hard work!

But more generally, I'm honestly not sure what support for custom lints would look like. Every time I think about what would be required in order to support custom lints I come to the same conclusion: it would look exactly like what we already support with analyzer plugins. Unless we could do something that would make it easier for users (and that can't be applied to the analyzer plugin support to also make it easier) I don't think we're likely to do anything different.

I doubt many of us have tried using (or were aware of) analyzer plugins, so now I know to give that shot, thanks! There's no wonder it's hard to tell where this feature lands. Dart is broken up into so many packages.

I generally discourage people from using our plugin support because we have not finished evaluating the impact that plugins have on the performance of the analysis server, nor are plugins used by the command-line analyzer.

I think the community at-large should try this anyway. If we don't have people trying to use it, we may not understand the performance impact or the desired experience for creating linter plugins.

The alternative is to write your own linter-like support using the analyzer package. It won't give you the IDE integration that analyzer plugins were designed to provide, but you could, at least, run them from the command-line and as part of your CI system.

From my perspective, there are so many tools in and adjacent-to the Dart SDK that it's hard to know which package/codebase to properly investigate for issues and solutions. I still can only guess how pedantic works, probably a hard-coded list of privately defined lints inside of the analyzer? The repo has one dart file that's 90% comments.

I'm very thankful for the language support we have thus far. I'm really looking forward to it catching up to being as developer friendly as ESLint. I'm no Javascript zealot, but I'm shocked at how much I missed auto-fix when working with Dart. There is a class of lints (I'd argue) that have one reasonable fix, so why report them at all? Why not just fix them? Things like prefer_single_quotes, annotate_overrides, prefer_const_constructors etc... I realize that functionally the linter should not be manipulating source-code but that's the experience I'm after.

[bwilkerson]

Thanks for the thoughtful comments.

I think the community at-large should try this anyway. If we don't have people trying to use it, we may not understand the performance impact or the desired experience for creating linter plugins.

That's probably true. If you do decide to give that path a try, please let us know how we could improve the support. As I said, it isn't being used much, as far as I know, so I'm sure there's lots of room for improvement.

From my perspective, there are so many tools in and adjacent-to the Dart SDK that it's hard to know which package/codebase to properly investigate for issues and solutions.

That's totally understandable. But when you find such a problem you can open an issue in the sdk repository and we'll be happy to route it to the correct team, including moving it to the correct repository if necessary. Please don't let the confusion of packages and repositories stop you from providing feedback or asking questions.

I still can only guess how pedantic works, probably a hard-coded list of privately defined lints inside of the analyzer? The repo has one dart file that's 90% comments.

The pedantic package defines several versions of an analysis options file that users can include in their own analysis options file. This file encodes the lints that are enabled internally at Google (but not necessarily by either the Dart or Flutter teams). So, no, there's no hard coded list; all of the lint rules that you enable by using pedantic are included in the analysis options file that pedantic provides.

One of the internal decisions was to disallow the use of ignore comments. However, one lint rule (unawaited_future) has some valid exceptions that can't be statically accounted for, so it was decided that we should support using a special function (unawaited) as a way to silence that particular lint. Because it was introduced for internal use we decided to define it in the pedantic package, which is what's in that one dart file.

There's nothing special or magical about pedantic (other than the unawaited function, which probably should have been defined elsewhere). Anyone can publish their own set of lint rules for use by others.

I'm no Javascript zealot, but I'm shocked at how much I missed auto-fix when working with Dart. There is a class of lints (I'd argue) that have one reasonable fix, so why report them at all? Why not just fix them? Things like prefer_single_quotes, annotate_overrides, prefer_const_constructors etc... I realize that functionally the linter should not be manipulating source-code but that's the experience I'm after.

I'm not aware of any plans to fix lints without user action, but we are looking at ways to make it easier to apply fixes than by applying each fix individually. Maybe not everything you're looking for, but hopefully a step in the right direction.

[leecommamichael]

Thinking this is the right place for this; here are my thoughts after spending a couple hours getting an analyzer plugin working today.

Disclaimer: I have not managed to get it working yet, but I'm hopeful because my plugin appears (and crashes!) in the Analysis Server Diagnostics page. There's nothing here for me to go off of; to be fair, the documentation did say they weren't easy to debug.

Unrecorded error while starting the plugin.
#0      new CaughtException.withMessage (package:analyzer/exception/exception.dart:51:47)
#1      new CaughtException (package:analyzer/exception/exception.dart:45:14)
#2      PluginSession.start (package:analysis_server/src/plugin/plugin_manager.dart:934:28)

#3      PluginInfo.start (package:analysis_server/src/plugin/plugin_manager.dart:210:42)
#4      PluginManager.addPluginToContextRoot (package:analysis_server/src/plugin/plugin_manager.dart:330:38)
#5      PluginWatcher.addedDriver (package:analysis_server/src/plugin/plugin_watcher.dart:67:19)

Having written some editor extensions for VSCode, what I've read of the analyzer_plugin package feels natural in terms of declaring editor capabilities up-front in the plugin; in this case, declared by what mixins/methods your plugin has implemented. The manner of specifying edits is also familiar-seeming which is good, but I'll have to see what I think when I get it working.

It was a bit of a brain twist, and I'll admit I drew a diagram, to make sense of the package structure. This is new to me as my experience with Dart is writing Flutter applications and CLI scripts (not having developed a package for third-party use.)

I spent a fair amount of time looking through built_value's usage of analyzer_plugin, but contradictory to the tutorial, none of their analysis_options.yaml declare a plugin. There was chatter referencing Angular's usage of analyzer_plugin, but all I found were dead-links to archived repositories. I did find Angular's repo, but I haven't searched exhaustively for an analyzer plugin implementation.

I'm not aware of good examples to move with (built_value seems to be a bit against the grain of the tutorial). But at the end of the day, I've got the right structure 🤞 and it crashes.

[bwilkerson]

I spent a fair amount of time looking through built_value's usage of analyzer_plugin, but contradictory to the tutorial, none of their analysis_options.yaml declare a plugin.

The tutorial probably didn't make this clear enough, but the plugin declaration needs to be added to the analysis_options.yaml file in any package (the target package) that depends on the plugin's host. Users of your package have to explicitly opt in to using your plugin.

There was chatter referencing Angular's usage of analyzer_plugin, but all I found were dead-links to archived repositories. I did find Angular's repo, but I haven't searched exhaustively for an analyzer plugin implementation.

My understanding is that they have dropped support for the plugin.

But at the end of the day, I've got the right structure 🤞 and it crashes.

You can pass the --instrumentation-log-file=<path> argument to the analysis server when starting it up. That will cause it to write information every time it receives requests from the client, sends responses or notifications to the client, sends requests to the plugins, or receives information from the plugins. That might contain more information about what's causing the plugin to crash.

[shyndman]

Hey @leecommamichael and others,

I wanted to point out Moor's analyzer plugin as being a potentially useful example. They have a setup going where their plugin runs both as an analysis server plugin, and as a standalone binary (which makes testing pretty easy, presumably).

Implementation: https://github.com/simolus3/moor/tree/master/moor_generator

Some documentation: https://moor.simonbinder.eu/docs/using-sql/sql_ide/

There's a lot to go through, but it might help.

[fartem]

@leecommamichael I ran into with same issue when my own plugin was starting on Dart Analyzer Server. In my case, solution was to change relative path in pubspec.yaml file in bootstrap package to absolute path (like from ../../ to /Users/fartem/Projects/dart_analyzer_plugin). There is working plugin from Wrike, you can explore this plugin and find more information on getting started and working with the plugins API.

[SAGARSURI]

Is there any update when this feature gonna land? We are expanding our team and now we want to have our own rules to maintain consistency across the team.

[pq]

Hey @SAGARSURI. Unfortunately, no. It's something we've wanted to support for a long time but haven't had the resources to schedule.

[bwilkerson]

I know I've said this before, but in the absence of an easier solution there are still two alternatives available to you:

1. You can write a stand-alone tool based on the analyzer package that will produce diagnostics. It's not ideal because users will have to run the tool manually and the results won't be integrated into IDEs or other workflows.

2. You could write a plugin using the analyzer_plugin package (which is built on top of the analyzer package). It's also not ideal because it's a non-trivial amount of work to get to where it's returning results, but it would then be integrated with both IDEs and the command-line analyzer (dart analyze).

[myryq]

One more workaround, we can create custom analysis server snapshot with own rules, e.g lines_longer_than_120_chars

[bwilkerson]

True. This has the disadvantage that you probably need to use APIs that we don't guarantee to continue supporting, but it does get you something that works. And because the command-line analyzer (dart analyze) uses the analysis server to get the analysis results, these will also show up there.

[nosmirck]

I know I've said this before, but in the absence of an easier solution there are still two alternatives available to you:

1. You can write a stand-alone tool based on the analyzer package that will produce diagnostics. It's not ideal because users will have to run the tool manually and the results won't be integrated into IDEs or other workflows.
2. You could write a plugin using the analyzer_plugin package (which is built on top of the analyzer package). It's also not ideal because it's a non-trivial amount of work to get to where it's returning results, but it would then be integrated with both IDEs and the command-line analyzer (dart analyze).

Do you have an example for Option 2? I'd like to see an example of something simple so I can write my own, but so far I've not found any easy to follow instructions, not in the documentation or anywhere else.

[SixSheeppp]

Is there any updates now ? I am going to make the Flutter project more international.

But the stuck job is how to scan the hard code of descrption, such as Text("hello world") I have to generate a file to translate 'hello world' , but it's going to be thounds of lines to check :(

If can custom lint rules,may be I can show 'warning' to remind me