New Lint: avoid_using_api

[getBoolean]

Closes #81

This lint has a lot of edge cases. Tests are included to verify basic functionality.

Packages Added:

• glob: Determines which files an entry should apply to (better than Regex for paths)
• yaml: To convert YamlMap to Map for the list of entries

Usage

External code can be "deprecated" when there is a better option available:

custom_lint:
  rules:
    - avoid_using_api:
      severity: info
      entries:
        - class_name: Future
          identifier: wait
          source: dart:async
          reason: "Future.wait should be avoided because it loses type safety for the results. Use a Record's `wait` method instead."
          severity: warning

Result:

void main() async {
  await Future.wait([...]); // LINT
  await (...,).wait; // OK
}

Advanced Usage

Each entry also has includes and excludes parameters. These paths utilize Glob patterns to determine if a lint entry should be applied to a file.

For example, a lint to prevent usage of a domain-only package outside of the domain folder:

custom_lint:
  rules:
    - avoid_using_api:
      severity: info
      entries:
        - source: package:modddels
          excludes:
            - "**/domain/**.dart"
          reason: "MODDDELS is only intended to be used in the domain layer."

[solid-vovabeloded]

Good job! I've added a couple of suggestions here - please take a look.

[getBoolean]

I think that's everything!

I do think documentation is needed, it will be very useful to know what all the lints are and what triggers them. Documentation for lints options can be included alongside that.

[yurii-prykhodko-solid]

Looks good overall, great piece of work, @getBoolean!

Let's make the CI happy and we can merge.

[getBoolean]

I hope that is everything.

[yurii-prykhodko-solid]

Outstanding piece of work, @getBoolean!

[illia-romanenko]

Nice! Our CI still fails - it feels like we need to fix those issues before merging?

[getBoolean]

Not sure why its failing, I'll look into it when I get home from vacation in a week.

[getBoolean]

I've confirmed it working correctly on Windows and MacOS. It might be an issue somewhere in AvoidUsingApiLinter._matchesSource on Linux or CI causing the source check to fail.

[getBoolean]

Good thing I setup Windows/Linux dual boot already, made tracking down the issue a lot easier.

The issue was that node.sourceUrl would return the filepath on Linux for project files, while it was always the package path on Windows/MacOS. In the documentation, maybe we should specifically state that doing source: package:myapp will not work on Linux and should be avoided. I updated the lint_test code to move the banned code to another package.

I also found two edge cases while debugging and fixed them

• lint for method usage on variable of a banned class type (source was previously ignored for it)
• lint for variable declaration of a banned class type if the type was inferred.

[yurii-prykhodko-solid]

In the documentation, maybe we should specifically state that doing source: package:myapp will not work on Linux and should be avoided.

Which makes sense -- that's what 'Deprecated' is for.

Let's add this as a note in code documentation, I think the AvoidUsingApiEntryParameters docs are the right place.

[yurii-prykhodko-solid]

Let's add this as a note in code documentation, I think the AvoidUsingApiEntryParameters docs are the right place.

Done.

@getBoolean Let me know if you're happy with my changes, then we can finish this up.

[yurii-prykhodko-solid]

@getBoolean Looks like there's some more issues -- CI still in the red.

[getBoolean]

Before we merge, I want to investigate making _matchesSource take a non nullable parent source url, it was hiding a lint issue before.

@yurii-prykhodko-solid it looks like pub get needs to be run on those test packages in CI, is that possible?

[yurii-prykhodko-solid]

@yurii-prykhodko-solid it looks like pub get needs to be run on those test packages in CI, is that possible?

Sure -- have a stab at our CI file. A find -> exec or a find -> for should do the trick.

UPD: Happened to need this on a different project, CI now fetches dependencies for all pubspecs it can find.

[getBoolean]

Thanks, I think that is everything now