solid_lints icon indicating copy to clipboard operation
solid_lints copied to clipboard

avoid_using_api: Lint Default Constructor Only

Open getBoolean opened this issue 1 year ago • 10 comments

While using the lint I noticed I missed an edge case, it is not possible to lint only the default constructor. it will always lint all usages of the class

Potential usage:

custom_lint:
  rules:
    - avoid_using_api:
      entries:
        # Add parentheses to lint only default constructor, use `identifier` for named constructors
        - class_name: Border()
          source: package:flutter
          reason: Use `BorderDirectional` instead.

Not sure if this is the best way to implement it, and I could see it being confusing if named constructors (ie class_name: Border.all()) didn't also work. Thoughts?

I can implement this.

getBoolean avatar Feb 12 '24 07:02 getBoolean

I also want to explore linting constructor parameters

getBoolean avatar Feb 12 '24 17:02 getBoolean

Maybe something like that?

custom_lint:
  rules:
    - avoid_using_api:
      entries:
        # Add parentheses to lint only default constructor, use `identifier` for named constructors
        - class_name: Border
          method: ()
          source: package:flutter
          reason: Use `BorderDirectional` instead.

and then

custom_lint:
  rules:
    - avoid_using_api:
      entries:
        # Add parentheses to lint only default constructor, use `identifier` for named constructors
        - class_name: Border
          method: all()
          source: package:flutter
          reason: Use `BorderDirectional` instead.

illia-romanenko avatar Feb 12 '24 17:02 illia-romanenko

In your example, is method a new option in addition to identifier? Or did you mean to use identifier?

I think that would work better.

for linting method parameters, I was thinking we could do something like this:

custom_lint:
  rules:
    - avoid_using_api:
      entries:
        - class_name: Border
          identifier: (left)
          source: package:flutter
          reason: Use `BorderDirectional(start)` instead.

getBoolean avatar Feb 12 '24 19:02 getBoolean

Yes - that should be it. Not sure what left means here but just () should be the best.

illia-romanenko avatar Feb 15 '24 17:02 illia-romanenko

left was a constructor parameter in the default constructor. Seems a bit confusing putting it in parentheses, so maybe a separate input field parameter would make it clear

The intent for my second comment was to only lint usage of a specific parameter in a constructor (or method)

final border = const Border(left: 4.0); // LINT
final border2 = const Border(top: 4.0); // Okay
custom_lint:
  rules:
    - avoid_using_api:
      entries:
        - class_name: Border
          identifier: ()
          parameter: left
          source: package:flutter
          reason: Use `BorderDirectional(start)` instead.

getBoolean avatar Feb 15 '24 18:02 getBoolean

Oh that's interesting! What if multiple parameters? It could be named named_parameter(s) as they are only variable, wdyt?

illia-romanenko avatar Feb 15 '24 20:02 illia-romanenko

I'm not sure how much benefit having multiple named parameters in a lint entry would provide. In my opinion, it's best if the lint reason is as specific as possible and should say what to use as a replacement. Linting multiple named parameters (while convenient) would require a more general lint reason, it wouldn't be able to say exactly what should be changed.

I'm in favor of it being named named_parameter, that should make it clear it doesn't work for positional parameters.

getBoolean avatar Feb 15 '24 21:02 getBoolean

I think it's settled then, we can return to multiple named parameters if the need arises.

illia-romanenko avatar Feb 17 '24 01:02 illia-romanenko

Instead of empty parentheses, I think it would be better to use the new keyword

custom_lint:
  rules:
    - avoid_using_api:
      entries:
        - class_name: Border
          identifier: new
          named_parameter: left
          source: package:flutter
          reason: Use `BorderDirectional()` instead.

getBoolean avatar Feb 18 '24 08:02 getBoolean

Since new is usually omitted - it feels like empty parentheses would be more understandable.

illia-romanenko avatar Mar 13 '24 00:03 illia-romanenko