Lint proposal: prefer static member function

[stephane-archer]

suggest all member functions that can be static

example

class foo {
   void foo(String a) {
       print(a);
   }
}

suggest:

class foo {
   static void foo(String a) {
       print(a);
   }
}

[bwilkerson]

Personally I'd rather have a lint to suggest converting such functions to extension methods on the first argument:

extension StringExtension on String {
  void foo() {
    print(this);
  }
}

Whether or not we have any lints like these, we should add assists for both transformations.

[lrhn]

This sounds like something that cannot be determined locally.

If some other library calls foo.foo("a") then it cannot be static. That's impossible to know.
If the function references a type parameter, it cannot be static. That may be visible in the code, not necessarily in the signature.

[stephane-archer]

@lrhn let me rephrase to make sure I understood correctly,
moving to:

class Foo {
   static void foo(String a) {
       print(a);
   }
}

would also mean modifying all calls like foo.foo("a") to Foo.foo("a") and result in an API breaking change for your library because an instance of a class can't access "static member function"

finding all references to foo in your codebase can be done by the editor, so I don't think this is an issue here.

Could you clarify, when a function can locally become static and will result in something impossible for the caller? instance.call() can always be turned into Type.call() from my understanding, does I miss something?

[bwilkerson]

I believe that the comments from @lrhn are more related to the assist I suggested than the lint rule. We could certainly flag all instance methods that don't refer to this (either explicitly or implicitly) and leave it up to the developer to add ignore comments where the method is required to be an instance method.

[lrhn]

If you convert a method to be a static function and all calls to be static function calls, then it should keep working - as long as nobody overrides the method. And as long as you can reference the type, which you should be for non-pathological code.
(Don't leak private types in your API. If you do, people may get objects of types that they cannot reference by name. That was always a bad idea, so I don't expect that to happen.)

That obviously assumes that the method can be static (doesn't reference this or type variables) and that it's not intended to be overridden. The last part is the one that's not decidable locally.

[bwilkerson]

That obviously assumes that the method can be static (doesn't reference this or type variables) and that it's not intended to be overridden. The last part is the one that's not decidable locally.

Good point. And because lints can't see anything beyond the local context, such a lint might well have too many false positives to be useful.

[stephane-archer]

@bwilkerson From my understanding, only functions that could be static but are intended to be overridden would make a false positive.
Because Lint rules are optional, the user will be judged if this happens a lot or not in their codebase.
In my codebase, it rarely happens, if something needs to be overridden then it's an abstract method.