Deprecated libraries and completion

[FMorschel]

In the SDK, I found a deprecated library which had a similar case to the following:

@Deprecated('Deprecated library')
library;

class MyDeprecatedClass {}

It was then exported by another library, like (consider it other.dart):

// ignore: deprecated_member_use_from_same_package
export 'deprecated.dart';

And when I had the last library already imported, no diagnostic was shown but I could still see the strokes on the completion:

---

IRL, the files were:

• Deprecated pkg/analyzer/lib/src/error/codes.g.dart
• Exporter pkg/analyzer/lib/src/error/codes.dart
• Importer pkg/analysis_server/lib/src/services/correction/fix_internal.dart

The deprecated library had this under deprecated:

  // This library is deprecated to prevent it from being accidentally imported
  // It should only be imported by the corresponding non-code-generated library
  // (which suppresses the deprecation warning using an "ignore" comment).
  'Use package:analyzer/src/error/codes.dart instead',

But as you can see, it still feels a bit weird if you look at the completion.

My question would be if we should stop showing deprecated for completions because the declaring library is deprecated?

CC @bwilkerson @DanTup

[FMorschel]

Maybe we should adhere to showing this whenever the declaration itself is deprecated or the library in question is.

For example, if I declare A in a.dart (not deprecated) and export it through b.dart, completion shows normally, and when I do complete selecting b.dart, I get a new diagnostic that the import library is deprecated.

[DanTup]

#52163 is possibly a little related to this.

I presume if you accept the completion here, you end up with:

• an import that is marked as deprecated
• a reference to the class that is not marked as deprecated

So on one hand this feels inconsistent (why is the class name deprecated in the completion but not in the editor?), but on the other hand if we don't show it that way on the completion, you'd have no hint at all that it's deprecated. Maybe we should show "(deprecated)" in front of the package URI on the right?

I'll admit I don't entirely understand the semantics of the code above though. Is the class deprecated, or only importing the library directly? (eg. if another library re-exports it and ignores the deprecated import, is it fine to use the class that way?).

[FMorschel]

#52163 is possibly a little related to this.

Yes, I missed it, thanks!

I presume if you accept the completion here, you end up with

Exactly. And when I do the opposite, deprecate an export and not the defining library, even though I could only see the export and never the original library (other project src) it show no sign of it being deprecated until I auto-complete it and it will add deprecated import.

Is the class deprecated, or only importing the library directly?.

From the comment I mentioned above, the analyzer team (and I) understand that only the defining library is deprecated; importing it from somewhere else would be fine.

Maybe we should show "(deprecated)" in front of the package URI on the right?

I don't think so... I'd say: if either the added import or the element being completed is deprecated, then we show deprecated as we do today (crossed), but if neither the element itself nor the exporting library is deprecated, then we show no sign of deprecation.

[DanTup]

I don't think so... I'd say: if either the added import or the element being completed is deprecated, then we show deprecated as we do today (crossed), but if neither the element itself nor the exporting library is deprecated, then we show no sign of deprecation.

Is this different to the behaviour we already have?

[FMorschel]

Is this different to the behaviour we already have?

Yes. Currently, we show as deprecated if either the element itself or the defining library is deprecated (see the screenshot at the OP). So all exports that are not deprecated should show as normal (see other.dart at OP).