Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid_using_api: Lint Default Constructor Only #126

Open
getBoolean opened this issue Feb 12, 2024 · 10 comments
Open

avoid_using_api: Lint Default Constructor Only #126

getBoolean opened this issue Feb 12, 2024 · 10 comments

Comments

@getBoolean
Copy link
Contributor

getBoolean commented Feb 12, 2024

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
Copy link
Contributor Author

getBoolean commented Feb 12, 2024

I also want to explore linting constructor parameters

@illia-romanenko
Copy link
Collaborator

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.

@getBoolean
Copy link
Contributor Author

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.

@illia-romanenko
Copy link
Collaborator

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

@getBoolean
Copy link
Contributor Author

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.

@illia-romanenko
Copy link
Collaborator

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

@getBoolean
Copy link
Contributor Author

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.

@illia-romanenko
Copy link
Collaborator

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

@getBoolean
Copy link
Contributor Author

getBoolean commented Feb 18, 2024

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.

@illia-romanenko
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants