Update dartanalyzer lint rule defaults to an empty set

[pq]

Part of #57873: update the lint registry defaults to line up with those defined by package:pedantic.

EDIT: as per conversation below, the new plan is to have no rules default to on.

[bwilkerson]

As I said elsewhere, I would prefer to see us remove the notion of "default" rules entirely. As far as I can tell, these defaults do not impact either the command-line analyzer or the analysis server. I'm not sure whether they impact the linter app, but I don't see any value to having them there. Do they bring some value that I'm not seeing?

[pq]

As far as I can tell, these defaults do not impact either the command-line analyzer

If you run dartanalyzer --lints and there is no .analysis_options.yaml, the defaults are what you get.

sdk/pkg/analyzer/lib/src/context/builder.dart

Lines 486 to 490 in cc9607b

	// If lints turned on but none specified, then enable default lints
	if (options.lint && options.lintRules.isEmpty) {
	options.lintRules = Registry.ruleRegistry.defaultRules;
	verbose('Using default lint rules');
	}

Initially the idea of a default set of rules was introduced to make trying out linting easier. (At that point no one had any experience w/ analysis_options, and technically may have predated them!) I think if the defaults are actually representative of some standard they may continue to add value but as they are today they are a distraction at best. An alternative would be to remove the default set entirely and I'm actually OK with that though I guess, technically, it'd be a breaking change?

[bwilkerson]

An alternative would be to remove the default set entirely and I'm actually OK with that though I guess, technically, it'd be a breaking change?

That's my preference. I don't know whether it would be a breaking change from a practical perspective. It would be nice to have analytics to tell us whether anyone runs the command-line analyzer with --lints. I suspect that the answer is 'no', but I have no proof.

On the other hand, changing the set of rules that we're running when someone does that is technically a breaking change whether we change it to a different non-empty set (such as pedantic) or to an empty set. So if we're going to make a breaking change, let's just remove the functionality.

[pq]

On the other hand, changing the set of rules that we're running when someone does that is technically a breaking change whether we change it to a different non-empty set (such as pedantic) or to an empty set.

Oh, I knew you'd say that. 😄

I guess I can go either way. I'd be curious what other folks think and happy to go with the flow. As long as we change the current set of defaults to something sensible (empty or pedantic), I'm good!

/cc @devoncarew @stereotype441

[devoncarew]

Being more restrictive would likely be a larger change from the POV of users than being less restrictive?

I'm not sure what lints we have enabled by default, but dropping down to an empty set of lints enabled if nothing is specified makes sense to me. People can then opt-into the lints they want, either deliberately by adjusting their analysis options, or through pedantic if they create a new project w/ stagehand.

[devoncarew]

I poked around the source a bit but couldn't see how or where the current default lints are defined.

[bwilkerson]

By using registerDefault when registering a lint, as here: https://github.com/dart-lang/linter/blob/master/lib/src/rules.dart#L182.

We might also want to make the --lints option print a message explaining how to upgrade if there are no lints defined because that would be an indication that someone was relying on the defaults.

[devoncarew]

Ah, thanks!

For posterity, here are the current lints enabled by default:

CamelCaseTypes
ConstantIdentifierNames
EmptyConstructorBodies
LibraryNames
LibraryPrefixes
NonConstantIdentifierNames
OneMemberAbstracts
SlashForDocComments
SuperGoesLast
TypeInitFormals
UnnecessaryBraceInStringInterps
UnnecessaryNullAwareAssignments
UnnecessaryNullInIfNullOperators

[pq]

SGTM.

I've update the title to reflect the new plan.

Thanks for the thoughtful conversation!