Add `strictCasts` API access to lints

[pq]

Follow-up from https://dart-review.googlesource.com/c/sdk/+/341336, with some more context in #53873.

A number of lints call typeSystem.isAssignableTo(...) which now supports a strictCasts flag. They have been updated to access strictCasts like this:

       strictCasts =
            // ignore: deprecated_member_use
            (context.analysisOptions as AnalysisOptionsImpl).strictCasts;

where context is a LinterContext instance.

Since we want to remove strictCasts from options, we need to put the access elsewhere. An easy candidate is the LinterContext but we should kick other ideas around.

Input welcome!

/cc @scheglov @srawlins @bwilkerson

[srawlins]

Can the parameter be made optional? I don't think any lint rules need to use the value of strict-casts.

[scheglov]

I see that LinterContext.analysisOptions is deprecated.
Not sure why.

Theoretically each file in the library could have its own analysis options, but I think we decided that we always use the one from the library defining unit. So, even though we could put AnalysisOptions get analysisOptions into LinterContextUnit, I think it is better to keep it in LinterContext. Well, maybe I miss something.

[scheglov]

Since we want to remove strictCasts from options

Do we? Why?

[pq]

Since we want to remove strictCasts from options
Do we? Why?

Huh. Now you've got me thinking. On second thought, perhaps we don't!

If it does stay in options, it'd be nice to not have to cast to AnalysisOptionsImpl so maybe better to add to AnalysisOptions.

I see that LinterContext.analysisOptions is deprecated.
Not sure why.

@srawlins: IIRC you did this in a clean up feeling it wasn't strictly necessary?

I feel pretty neutral between:

1. AnalysisOptions.strictCasts (and un-deprecating LinterContext.analysisOptions) and
2. LinterContext.strictCasts

(With maybe a mild preference for 1.)

What do you all think?

[srawlins]

I see that LinterContext.analysisOptions is deprecated.

Not sure why.

It seemed unneeded to me. A caller can use LinterContext.currentUnit.unit.declaredElement?.session.

What do you all think?

I don't think any lint rules need to use the value of strict-casts.

[scheglov]

Since we want to remove strictCasts from options
Do we? Why?

Huh. Now you've got me thinking. On second thought, perhaps we don't!

If it does stay in options, it'd be nice to not have to cast to AnalysisOptionsImpl so maybe better to add to AnalysisOptions.

Yes.

I see that LinterContext.analysisOptions is deprecated.
Not sure why.

@srawlins: IIRC you did this in a clean up feeling it wasn't strictly necessary?

I feel pretty neutral between:

1. AnalysisOptions.strictCasts (and un-deprecating LinterContext.analysisOptions) and
2. LinterContext.strictCasts

(With maybe a mild preference for 1.)

What do you all think?

We are going to expose AnalysisOptions from FileResult, which is public API.
So, it is fine to expose it to the private APIs, e.g. linter.
So, (1).

Not using strictCasts at all, and relying on default does not seem right to me.
If it does not matter, just remove this option completely.
If it does matter, use what the user said.

[pq]

I don't think any lint rules need to use the value of strict-casts.

Not historically, but soon, with the strictCasts removal from type system objects we'll need this value anywhere we call isAssignableTo.

See for example:

https://dart-review.googlesource.com/c/sdk/+/341336/13/pkg/linter/lib/src/rules/use_super_parameters.dart

[pq]

We are going to expose AnalysisOptions from FileResult, which is public API.
So, it is fine to expose it to the private APIs, e.g. linter.
So, (1).

SGTM

[srawlins]

we'll need this value anywhere we call isAssignableTo.

I see strictCasts is optional at isAssignableTo. https://dart-review.googlesource.com/c/sdk/+/341336/13/pkg/analyzer/lib/src/dart/element/type_system.dart

Not using strictCasts at all, and relying on default does not seem right to me.
If it does not matter, just remove this option completely.
If it does matter, use what the user said.

It matters in some contexts. Not in any lint rules.