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

proposal: missing_code_block_language_in_doc_comment #4904

Closed
5 tasks
kallentu opened this issue Feb 29, 2024 · 4 comments
Closed
5 tasks

proposal: missing_code_block_language_in_doc_comment #4904

kallentu opened this issue Feb 29, 2024 · 4 comments

Comments

@kallentu
Copy link
Member

missing_code_block_language_in_doc_comment

Description

A fenced code block is missing a specified language.

Details

To enable proper syntax highlighting of Markdown code blocks, dart doc requires code blocks to specify the language used after the initial declaration.

Kind

It guards against syntax highlighting errors.

DO specify the language used in the code block of a doc comment.

Bad Examples

/// ```
/// void main() {}
/// ```
/// ~~~
/// void main() {}
/// ~~~

Good Examples

/// ```dart
/// void main() {}
/// ```
/// ~~~dart
/// void main() {}
/// ~~~

Discussion

This migrates Dartdoc's missingCodeBlockLanguage warning to the linter for the clean up of Dartdoc.

cc. @srawlins @goderbauer @pq

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.
@bwilkerson
Copy link
Member

The design of the lint looks fine, but I have a couple of related questions.

Would this replace the warning in dartdoc, or would it duplicate it?

Are there any plans for how to advertise the new lint to users of the current warning?

@srawlins
Copy link
Member

Would this replace the warning in dartdoc, or would it duplicate it?

Replace!

Are there any plans for how to advertise the new lint to users of the current warning?

I think to enable the warning, you have to specify it in the dartdoc options file; flutter has a line, commented out. So when we deprecate the dartdoc functionality, we can refer folks to the lint rule.

@bwilkerson
Copy link
Member

I like those answers, but they raised more. (Let me know if we should move this discussion elsewhere.)

How long after deprecating the warning in dartdoc will the warning need to continue to be supported? It used to be the case that many users ran dartdoc from the package (via pub global activate), not from the SDK. If users who are doing that upgrade dartdoc to a version that doesn't report the warning without upgrading their SDK to a version where analyzer can report the lint, they could be left without any warnings.

Will we need a breaking change announcement to let users know that they now need to run the analyzer in order to catch this issue? (I like to imagine that every user runs the analyzer, but I don't know that we can depend on that.)

@srawlins
Copy link
Member

How long after deprecating the warning in dartdoc will the warning need to continue to be supported?

30 days. Seems long enough to migrate.

If users who are doing that upgrade dartdoc to a version that doesn't report the warning without upgrading their SDK to a version where analyzer can report the lint, they could be left without any warnings.

True. I think true of any deprecation.

Will we need a breaking change announcement to let users know that they now need to run the analyzer in order to catch this issue? (I like to imagine that every user runs the analyzer, but I don't know that we can depend on that.)

No. It is not a breaking change to stop printing any given text from dart doc.

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Mar 11, 2024
This CL adds a lint that warns you if you are missing a language info string after the first code block fence.

This should replace Dartdoc's in-house `missingCodeBlockLanguage` warning.

Bug: dart-lang/linter#4904
Change-Id: Ie7279a8a0c041de6937ab0841c4a207b16064307
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/356282
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
kallentu added a commit to flutter/flutter that referenced this issue Apr 1, 2024
In preparation to add the lint
`missing_code_block_language_in_doc_comment`, added info strings to a
bunch of fenced code blocks.

Related to issue: dart-lang/linter#4904

## Pre-launch Checklist

- [X] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [X] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [X] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [X] I signed the [CLA].
- [X] I listed at least one issue that this PR fixes in the description
above.
- [X] I updated/added relevant documentation (doc comments with `///`).
- [X] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [X] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [X] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
[Data Driven Fixes]:
https://github.com/flutter/flutter/wiki/Data-driven-Fixes
kallentu added a commit to flutter/flutter that referenced this issue Apr 2, 2024
Part 2 from #146085
In preparation to add the lint
`missing_code_block_language_in_doc_comment`, added `none` info strings
to a bunch of fenced code blocks that have miscellaneous text or output
text.

Related to issue: dart-lang/linter#4904

## Pre-launch Checklist

- [X] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [X] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [X] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [X] I signed the [CLA].
- [X] I listed at least one issue that this PR fixes in the description
above.
- [X] I updated/added relevant documentation (doc comments with `///`).
- [X] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [X] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
[Data Driven Fixes]:
https://github.com/flutter/flutter/wiki/Data-driven-Fixes
kallentu added a commit to flutter/flutter that referenced this issue Apr 3, 2024
Adds this Dartdoc lint to the flutter repository, in replacement of the
warning it used to have.

Lint Proposal: dart-lang/linter#4904

## Pre-launch Checklist

- [X] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [X] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [X] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [X] I signed the [CLA].
- [X] I listed at least one issue that this PR fixes in the description
above.
- [X] I updated/added relevant documentation (doc comments with `///`).
- [X] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [X] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [X] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
[Data Driven Fixes]:
https://github.com/flutter/flutter/wiki/Data-driven-Fixes
kallentu added a commit to flutter/engine that referenced this issue Apr 8, 2024
…ne. (#51944)

Adds this Dartdoc-related lint to the flutter repository, in replacement
of the Dartdoc warning (`missingCodeBlockLanguage`) because it will be
deprecated and removed soon.

flutter/flutter already has this lint as well.

Lint Proposal: dart-lang/linter#4904

## Pre-launch Checklist

- [X] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [X] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [X] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [X] I added new tests to check the change I am making or feature I am
adding, or the PR is [test-exempt]. See [testing the engine] for
instructions on writing and running engine tests.
- [X] I updated/added relevant documentation (doc comments with `///`).
- [X] I signed the [CLA].
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
mockturtl added a commit to mockturtl/tidy that referenced this issue May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants