Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unused private constructor doesn't trigger unused_* #49654

Closed
matanlurey opened this issue Aug 13, 2022 · 10 comments
Closed

Unused private constructor doesn't trigger unused_* #49654

matanlurey opened this issue Aug 13, 2022 · 10 comments
Labels
analyzer-warning Issues with the analyzer's Warning codes area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@matanlurey
Copy link
Contributor

Minimal reproduction:

void example(PublicClassPrivateConstructor? e) {
  print(e == null);
}

class PublicClassPrivateConstructor {
  PublicClassPrivateConstructor._();
}

I'm guessing there is an argument this is often a "hint" to prevent extends or with, which leads me to believe we need an @interface annotation - but it's also probably reasonable to use // ignore: unused_constructor for this purpose as well.

@matanlurey
Copy link
Contributor Author

Interestingly enough, adding a factory constructor starts triggering:

void example(PublicClassPrivateConstructor? e) {
  print(e == null);
}

class PublicClassPrivateConstructor {
  factory PublicClassPrivateConstructor() {
    throw UnimplementedError();  
  }
  
  PublicClassPrivateConstructor._();
}

@srawlins
Copy link
Member

Yeah it's intentional for the reasons you give.

@srawlins
Copy link
Member

Want to change this to a request for @interface?

@srawlins srawlins added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request labels Aug 13, 2022
@matanlurey
Copy link
Contributor Author

@srawlins I'm OK with you closing this as a dupe of #46331 in that case. Up to you.

@srawlins
Copy link
Member

srawlins commented Apr 4, 2023

I'll keep this open as we should change the behavior soon for Dart 3.0 interfaces.

@srawlins srawlins added the type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) label Mar 14, 2024
@srawlins srawlins added the analyzer-warning Issues with the analyzer's Warning codes label Aug 16, 2024
@lrhn
Copy link
Member

lrhn commented Aug 16, 2024

It's not just a hint, it prevents the class from having an implicit public default constructor.
That means that having a single private constructor is a reasonable thing to do, even if nobody uses it.
(Today abstract interface can do the same thing - then it doesn't matter that the class has a default constructor, if you can't call it and can't use it as a super constructor. Used to be that the private constructor was the best way to achieve that.)

@srawlins
Copy link
Member

Alright let's call it meaningful then.

@matanlurey
Copy link
Contributor Author

Alright let's call it meaningful then.

I have an inverse read of the situation, now that we have abstract interface, an unused private constructor is dead code :P.

@bwilkerson
Copy link
Member

While I see your perspective, the code serves a purpose. There's another (and maybe better) way to accomplish the same purpose, but that doesn't negate the fact that the code can't just be removed without making other changes.

The two styles are both valid Dart, and if would be irritating to a user that preferred the constructor approach to have diagnostics that they never intended to resolve.

I could argue that the lint ought to flag such a constructor if the class is already marked as abstract interface. I don't know whether we do.

In addition, we could have a lint to flag cases where a class could be marked with abstract interface. If we did add such a lint it should have a fix.

Or we could skip the lint and implement an assist that would convert the class and remove the constructor. It isn't clear to me that anyone would discover such an assist, however.

@lrhn
Copy link
Member

lrhn commented Aug 19, 2024

If you can guarantee that the default constructor of an abstract interface class is not shown in DartDoc, then I'll agree that there is a better way. (But also don't want to migrate too much code for that, and we still do allow pre-3.0 code to exist, so unless the lint can behave differently for pre-3.0 code than for post-3.0 code, it's a problem.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-warning Issues with the analyzer's Warning codes area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

4 participants