Skip to content

Support mustBeOverriden on constructors #53357

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

Open
Nexushunter opened this issue Aug 26, 2023 · 2 comments
Open

Support mustBeOverriden on constructors #53357

Nexushunter opened this issue Aug 26, 2023 · 2 comments
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-pkg-meta Issues related to package:meta P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@Nexushunter
Copy link

Nexushunter commented Aug 26, 2023

Thank you for taking the time to file an issue!

This tracker is for issues related to:

Dart Core library: meta

Dart info:

General info

[✓] Flutter (Channel stable, 3.13.1, on Manjaro Linux 6.1.44-1-MANJARO, locale en_CA.UTF-8)
• Flutter version 3.13.1 on channel stable at /home/nexus/snap/flutter/common/flutter
• Upstream repository https://github.com/flutter/flutter.git
• Framework revision e1e47221e8 (4 days ago), 2023-08-22 21:43:18 -0700
• Engine revision b20183e040
• Dart version 3.1.0
• DevTools version 2.25.0

Project info

  • sdk constraint: '>=3.0.5 <4.0.0'
  • dependencies: bloc, build_runner, built_collection, built_value, cupertino_icons, dio, equatable, flutter, flutter_bloc, go_router, logging, meta, provider
  • dev_dependencies: bloc_test, flutter_lints, flutter_test, http_mock_adapter, mockito
  • elided dependencies: 1

Process info

Memory CPU Elapsed time Command line
1074 MB 5.4% 01:21:51 dart language-server --client-id=Android-Studio --client-version=AI-223.8836.35 --protocol=analyzer
213 MB 0.5% 35:33 dart language-server --client-id=WebStorm --client-version=WS-232.9559.58 --protocol=analyzer
109 MB 0.1% 04:04:11 flutter_tools.snapshot daemon
100 MB 0.1% 01:21:43 flutter_tools.snapshot daemon

My use case is as follows:

I have a series of objects generated from an OAS spec and there are some common functions I'd like to do on the types they get converted into. To provide some of this in an abstracted manner I'm providing the following:

abstract class ApiObject<T> extends Equatable {
  const ApiObject();

  @visibleForTesting
  @mustBeOverridden
  T toClientObject();

  @mustBeOverridden 
  const ApiObject.fromClientObject(T object) : this();

  @mustBeOverridden
  @override
  List<Object> get props => [];
}

and the conversion object looks like:

class Metadata extends ApiObject<client.UrlMetadata> {
  final String title;
  final String? image;
  const Metadata({this.title = '', this.image});

  @override
  List<Object> get props => [title, image ?? ''];

  @override
  client.UrlMetadata toClientObject() => (client.UrlMetadataBuilder()
        ..title = title
        ..image = image)
      .build();
}

The problem is with:

  @mustBeOverridden 
  const ApiObject.fromClientObject(T object) : this();

The analyzer throws the warning:

The annotation 'mustBeOverridden' can only be used on fields, getters, methods, or setters.

and the Metadata class doesn't throw an analyzer error for the missing constructor (which is expected given the current usage).

Proposal:
Allow for this annotation to also be applied to constructors to allow for enforcement of this requirement.

@bwilkerson
Copy link
Member

It looks like you're trying to require that every subclass of ApiObject define a constructor with the name fromClientObject. While this is a reasonable convention to have, I don't think that mustBeOverridden is the right way to enforce that convention.

The reason is because the notion of overriding only applies to instance members, and constructors are not instance members. I think it would be confusing to use the term 'override' to mean something that isn't an override.

Unfortunately, we don't currently have any support for enforcing the convention you're using. I'm also not sure how widely used such a feature would be. Are you aware of any precedence for such a feature from other languages?

@bwilkerson bwilkerson added the area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). label Aug 26, 2023
@Nexushunter
Copy link
Author

It looks like you're trying to require that every subclass of ApiObject define a constructor with the name fromClientObject. While this is a reasonable convention to have, I don't think that mustBeOverridden is the right way to enforce that convention.

That is correct. I think Java maybe the only one (not that I'm coming from a java background).

Since this convention might not be the best way to enforce this, what would be your suggestion? I don't think a static method would be productive or even feasible, given that static implies that it is the only method like that but if that is preferred I would be ok with that, in this case.

I'm currently implementing it in each class but I'm not sure what would be the best dart convention for this. Thanks for the rapid reply

@lrhn lrhn added legacy-area-analyzer Use area-devexp instead. devexp-pkg-meta Issues related to package:meta and removed area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). labels Aug 27, 2023
@keertip keertip added type-enhancement A request for a change that isn't a bug P3 A lower priority bug or feature request labels Aug 28, 2023
@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-pkg-meta Issues related to package:meta P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants