Skip to content

public_member_api_docs should not require docs for trivial constructors #57361

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
yjbanov opened this issue Aug 5, 2016 · 9 comments
Open
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. customer-flutter devexp-linter Issues with the analyzer's support for the linter package linter-false-positive P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@yjbanov
Copy link

yjbanov commented Aug 5, 2016

There are situations when doc comments are not useful in a constructor:

The following is common in classes used as annotations:

/// Docs.
class Foo {
  /// Not much to say here but public_member_api_docs insists.
  Foo();
}

Even when the constructor has parameters, if the parameters assign to public fields that are documented, there isn't much to say in the constructor:

/// Docs.
class Foo {
  /// Not much to say here but public_member_api_docs insists
  Foo(this.bar, this.baz);

  /// Docs.
  final bar;

  /// Docs.
  final baz;
}

@pq @Hixie

@Hixie
Copy link
Contributor

Hixie commented Aug 5, 2016

We should still say something. And that something shouldn't be trivial.

For example, it should talk about whether bar or baz can be null, what requirements there are on their values, what constructing one of these objects implies, what the lifecycle will be, whether you're required to dispose of the object, whether the object is 'const', etc.

@yjbanov
Copy link
Author

yjbanov commented Aug 8, 2016

it should talk about whether bar or baz can be null, what requirements there are on their values

These would be documented in the respective fields' doc comments.

what constructing one of these objects implies, what the lifecycle will be, whether you're required to dispose of the object

These would be documented in the class' doc comment.

whether the object is 'const'

The constructor must have the const modifier in this case. Whether the class is intended to be used as a constant, e.g. it's an annotation, would also be documented in the class' doc comment.

Flutter can require to document every constructor in its own code base. In general though, requiring it is more likely to lead to filler doc comments than good documentation.

If enough projects consider documenting trivial constructors important, then perhaps the linter could provide more knobs, e.g. require_docs_on_trivial_constructors in addition to public_member_api_docs.

@Hixie
Copy link
Contributor

Hixie commented Aug 8, 2016

I agree that relevant documentation should also be in the field docs, and indeed in some cases in the class docs. But it should also be in the constructor docs. The constructor docs are what you see in your IDE when you're typing the constructor, for instance. Users shouldn't have to chase down every last reference just to know how to use a constructor.

A class can have a mix of const constructors and non-const constructors. In that case, it can be useful for the non-const constructors to discuss the const constructors and describe the implications of using one instead of the other.

I agree that we shouldn't have filler docs. (Filler docs are a violation of Flutter' style guide.)

I'm fine with having more knobs. My concern here is only over whether we are able to configure Flutter's analysis the way we want it to get good results.

@pq pq added type-enhancement A request for a change that isn't a bug customer-flutter labels Aug 16, 2016
@pq
Copy link
Member

pq commented Aug 16, 2016

Another option (not supported yet) would be to make the lint parametric. Then we could say something like:

rules:
  public_member_api_docs
    require_docs_on_trivial_constructors: true

But anyway...

Focusing on the now, it'd be great to get a sense for what we need to do minimally to make public_member_api_docs useful in the context of flutter. Thoughts?

@yjbanov, @Hixie : is changing this behavior a blocker for flutter?

@Hixie
Copy link
Contributor

Hixie commented Aug 16, 2016

No, the current situation is workable for us.

@pq
Copy link
Member

pq commented Aug 16, 2016

Cool. Thanks.

@yjbanov
Copy link
Author

yjbanov commented Aug 16, 2016

I like the parametric option.

@LuisMiguelSS
Copy link

LuisMiguelSS commented Sep 13, 2023

Any plans for considering this?

it should talk about whether bar or baz can be null, what requirements there are on their values

As @yjbanov mentioned, it could be documented, but even nowadays we have Sound null safety, where we strictly indicate whether or not a value can be null, so it's no longer necessary to document such case.

While it sometimes might be interesting to document more complex constructors, for most of the cases we already know what they will do.

I also think that the parametric solution would be desirable 🤞

@srawlins
Copy link
Member

I don't think there is good consensus on any new rules here, except that configurability would be good :)

@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 18, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 18, 2024
@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 21, 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. customer-flutter devexp-linter Issues with the analyzer's support for the linter package linter-false-positive 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

7 participants