Revisit `avoid_classes_with_only_static_members`

[minhqdao]

Proposal

I think we should revisit avoid_classes_with_only_static_members.

Imagine the following case:

// linter: Avoid defining a class that contains only static members
class ProjectColors {
  static Color get primary => Colors.blue;
  static Color get secondary => Colors.white;
}

But should you really avoid classes that contain only static members? Well, no. You should avoid instantiable and extendable classes that contain only static members. Therefore, this is what you actually want to have:

// linter is happy
class ProjectColors {
  ProjectColors._();

  static Color get primary => Colors.blue;
  static Color get secondary => Colors.white;
}

And this is also exactly how the material library has its Colors class defined.

The current linting rule, however, suggests:

"Creating classes with the sole purpose of providing utility or otherwise static methods is discouraged. Dart allows functions to exist outside of classes for this very reason."

What's wrong with my utility class that serves as a namespace for my project colors? I think there's nothing wrong with it. Do I really want to refactor this utility class into functions? No, I don't. What is the problem with utility classes in general? I don't think there aren't any as long as the classes cannot be instantiated or extended from outside.

I therefore propose to change the details and description of the linting rule and further add an example so developers know how to include a private, named constructor to make it non-instantiable and -extendable from outside the class.

I can offer to open a PR and make the changes. Also I'm thinking about trying to add a "quick fix" where a private, named constructor is automatically added.

As this is also part of the effective dart guide, maybe we want to rethink that one, too. 🤔 If not, we should still consider changing the linting behavior. Because if the rule is to be taken seriously, it should not be satisfied by the presence of a private constructor.

What do you guys think?

[bwilkerson]

Many of the lints, this one included, exist to support stylistic choices made by individuals or teams. As you noted, this one exists to support the section from the Dart style guide that says

AVOID defining a class that contains only static members.

In the discussion of the guideline, it also states

However, this isn’t a hard rule. With constants and enum-like types, it may be natural to group them in a class.

The reason the lint doesn't currently support this exception is that, as worded, there isn't any way for a static analysis tool to be able to identify the times when it does and doesn't make sense to group them into a class.

It isn't clear to me that there is any way to statically determine when an exception makes sense without additional information. In particular, I'm not convinced that defining a private constructor is a sufficient signal. (I'm also not entirely convinced that it isn't.)

But I am convinced that marking the class with an ignore comment is a sufficient signal. Doing so has the added benefit that the intent is made explicit so that a code reviewer can know that they need to make a determination as to whether this particular use case is a valid exception to the rule. I don't believe that having a private constructor would be as clear a signal to code reviewers that an agreed upon style is being intentionally violated.