Skip to content

proposal: library_with_comments_or_annotations #58871

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

Closed
5 tasks done
kevmoo opened this issue Sep 14, 2022 · 7 comments
Closed
5 tasks done

proposal: library_with_comments_or_annotations #58871

kevmoo opened this issue Sep 14, 2022 · 7 comments
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. linter-lint-proposal linter-status-pending

Comments

@kevmoo
Copy link
Member

kevmoo commented Sep 14, 2022

library_with_comments_or_annotations

Description

Doc comments and/or annotations that are meant to be "scoped" to a library should be attached to a library; directive and not just "floating".

Details

We have a bunch of silly rules trying to deal with doc comments and annotations "floating" that are meant to be scoped to a library (and not the first import/export directive).

Preferring these to be connected to a library directive would make their purpose clear and likely help formatting and code analysis tools.

Kind

style

Good Examples

/// This is a cool library doc comment
@WithAnAnnotation()
library; // assuming https://github.com/dart-lang/language/issues/1073 lands with support for no-name library directives

import 'dart:async';

Bad Examples

/// This is a bad floating library doc comment
@WithAFloatingAnnotation()

import 'dart:async';

Discussion

This is the ONE case where https://github.com/dart-lang/linter/issues/3686 (prefer avoiding library directives) would not apply!

Adding no-name libraries proposal: dart-lang/language#1073

This could likely be added to Effective Dart, I think.

Discussion checklist

  • List any existing rules this proposal modifies, complements, overlaps or conflicts with.
  • List any relevant issues (reported here, the SDK Tracker, or elsewhere).
  • If there's any prior art (e.g., in other linters), please add references here.
  • If this proposal corresponds to Effective Dart or Flutter Style Guide advice, please call it out. (If there isn’t any corresponding advice, should there be?)
  • If this proposal is motivated by real-world examples, please provide as many details as you can. Demonstrating potential impact is especially valuable.
@srawlins
Copy link
Member

The annotation bit might be tricky; it might be intentional. But the doc comment case I think is unambiguous.

@srawlins
Copy link
Member

Perhaps if we only complain about annotations on the first directive (like an import), our false negatives will be extremely minimal. And then not worry about annotations on the first member (like main or something).

@kevmoo
Copy link
Member Author

kevmoo commented Sep 14, 2022

Perhaps if we only complain about annotations on the first directive (like an import), our false negatives will be extremely minimal. And then not worry about annotations on the first member (like main or something).

Or (likely more difficult) lint when they are "floating" – with newlines below them.

@bwilkerson
Copy link
Member

We could also flag for annotations whose target kind is 'library' because that's an unambiguous signal.

@kevmoo
Copy link
Member Author

kevmoo commented Oct 26, 2022

FYI: the associated language feature has LANDED for Dart 1.19!

@kevmoo kevmoo changed the title proposal: prefer_library_with_comments_or_annotations proposal: library_with_comments_or_annotations Oct 26, 2022
@kevmoo
Copy link
Member Author

kevmoo commented Oct 26, 2022

Dropped prefer_ prefix as is tradition

@srawlins
Copy link
Member

Sorry I missed this request. These are each complete now, with the library_annotations and dangling_library_doc_comments lint rules.

@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 19, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. linter-lint-proposal linter-status-pending
Projects
None yet
Development

No branches or pull requests

4 participants