proposal: `prefer_extension_accessor_to_top_level_function`

[srawlins]

prefer_extension_accessor_to_top_level_function

Description

Prefer an extension accessor (getter or setter) to a top-level function with one parameter.

Details

I am really enjoying extensions. There are many static or top-level functions which meet some simple criteria that are easily converted to an extension, in particular getters and setters, and result in code which is both more readable and more brief (not a super common combo!). Here are two quick examples:

void f(parameter) {
  if (parameter.intProperty.isOdd ||
      isMultipleOfTen(parameter.intProperty)) {
    print('good');
  }
}

bool isMultipleOfTen(int i) => i % 10 == 0;

// can be:

void f(parameter) {
 if (parameter.intProperty.isOdd ||
     parameter.intProperty.isMultipleOfTen) {
    print('good');
  }
}

extension on int {
  bool get isMultipleOfTen => this % 10 == 0;
}

void f(parameter) {
  print('parameter is ${getBinaryRepresentation(parameter.intProperty)}');
}

String getBinaryRepresentation(int i) => i.toRadixString(2);

// can be:

void f(parameter) {
  print('parameter is ${parameter.intProperty.binaryRepresentation}');
}

extension on int {
  String get binaryRepresentation => toRadixString(2);
}

This lint rule would certainly have a high rate of false positives, requiring an annoying amount (some would deem, an unacceptable amount) of // ignore comments. But we can narrow the requirements down I think to a fairly strict and benign set. These conditions would also help structure a quick fix:

1. Given a top-level function,
2. with exactly one parameter (can have an interface type, a function type, or even be a type variable)
3. and which meets one of these three conditions: a. returns a bool and is named has* or is* (others?) b. returns a non-void, non-Never (and non-Future<void> and non-FutureOr<void> etc ?) type, and is named get* c. returns a void, and is named set*

we recommend changing the function to an extension. The quick fix would apply as follows:

bool hasXXX(T parameter) => ...;

// ==>

extension on T {
  bool get hasXXX => ... /* replace each 'parameter' with 'this' */;

S getXXX(T parameter) => ...;

// ==>

extension on T {
  S get xXX => ... /* replace each 'parameter' with 'this' */;

void setXXX(T parameter) => ...;

// ==>

extension on T {
  void set xXX => ... /* replace each 'parameter' with 'this' */;

Kind

Style

Discussion

If we turn down the lint rule idea, the fix would still be great as a Refactoring.

Discussion checklist

• [ ] List any existing rules this proposal modifies, complements, overlaps or conflicts with.
• [ ] List any relevant issues (reported here, the SDK Tracker, or elsewhere).
• [ ] If there's any prior art (e.g., in other linters), please add references here.
• [ ] If this proposal corresponds to Effective Dart or Flutter Style Guide advice, please call it out. (If there isn’t any corresponding advice, should there be?)
• [ ] If this proposal is motivated by real-world examples, please provide as many details as you can. Demonstrating potential impact is especially valuable.

[bwilkerson]

I am concerned about the number of false positives. Unless we can reduce the false positives I don't think this would be a good candidate for a lint.

But I agree that this would make a good refactoring, and I've suggested it before (in an internal forum, so I can't link to it here).

[srawlins]

Yeah I think I am mostly motivated to have the assist / quick-fix. I agree there is sort of no way to prove that codebases wouldn't be rife with false positives, just because functions are named a certain way.

The downside to an assist is that I couldn't make a sweeping change, but that's probably not so sad; if the goal is to improve readability here and there, well, code is only un-readable if I'm looking at it, and if I am bothered by it, then an assist would be great; a preemptive lint + quick-fix is not necessary.

[lrhn]

If this is restricted to top-level functions named getX and setX, it's probably fairly safe. Those are badly named already, and could use some help.

Should it apply to _getX and _setX as well, or is it only for public APIs?

It could apply to static methods as well as top-level functions, but it gets weird when the argument type is not the type the surrounding class. There might be a reason for the scope. (And if it is on the surrounding class, they could have added the instance method if they wanted to, so there is probably a reason.)

Maybe there could be a quick-fix to convert any method to an extension method.

• An instance method directly to an extension methods.
• A static method/top-level function to an extension method on its first argument.
• Make that a getter or setter if named getX with one parameter or setX with two parameters, removing the get/set from the name.
• Make it a getter if named isX or hasX, one parameter and return type bool. (Or provide a separate fix to convert any static non-void unary method to a getter).

No lint required, but then you have to find the candidates yourself.

[srawlins]

Should it apply to _getX and _setX as well, or is it only for public APIs?

I think so, yes.

It could apply to static methods as well as top-level functions, but it gets weird when the argument type is not the type the surrounding class. There might be a reason for the scope. (And if it is on the surrounding class, they could have added the instance method if they wanted to, so there is probably a reason.)

All of these reasons have convinced me its best to leave static methods alone, haha.

Maybe there could be a quick-fix to convert any method to an extension method. [...] on its first argument.

This is a great idea. I think a great candidate.