[wip] Introduce DartDoc "Summary Line" lint #3820

ditman · 2022-11-07T22:06:57Z

This is a WIP lint to enforce that DartDoc comments start with a ~~brief (140 chars) paragraph~~ single line that ends in a period ('.').

If there's additional content on the DartDoc, it expects a blank line before continuing, so:

OK:

/// This is a valid dartdoc comment.

OK:

/// This is also a valid dartdoc comment.
///
/// With some extra information right below as the next paragraph.
/// The comment can now be as complex as needed here, but it's
/// important that the first line is single and ends in a period.

BAD:

/// This is a bad comment, because it
/// spills over two lines.

BAD:

/// This is a bad comment, because it doesn't end in a period!

BAD:

/// This is a bad comment, because it doesn't leave a blank space.
/// Between the first line and the rest of the content, that may be
/// arbitrarily complicated.

IGNORED:

/**
 * This is a JavaDoc style comment.
 *
 * And is ignored by the linter. This is discouraged, don't do it.
 */

This lint only validates that the first ~~"paragraph" exists, is brief, and~~ line is isolated and ends in a period. It doesn't attempt to do any extra validation, like measuring the length of the line. Additional validation is left to other linters.

This lint ~~supports both /// and /**~~ only supports \\\ comments.

Manual testing

I've run this lint against this repo, here's the CLI output:

dit@dit:~/github/dart-linter$ dart run bin/linter.dart --rules=dartdoc_summary_line ~/github/dart-linter/lib

/src/util/score_utils.dart 11:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// todo(pq): de-duplicate these fetches / URIs
^^^
/src/util/tested_expressions.dart 124:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// TODO: A truly smart implementation would detect
  ^^^
/src/util/flutter_utils.dart 57:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// See: analysis_server/lib/src/utilities/flutter.dart
^^^
/src/util/ascii_utils.dart 5:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// String utilities that use underlying ASCII codes for improved performance.
^^^
/src/util/condition_scope_visitor.dart 133:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// todo (pq): here and w/ getTrueExpressions, consider an empty iterable
  ^^^
/src/util/leak_detector_visitor.dart 40:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// Builds a function that reports the variable node if the set of nodes
^^^
/src/util/unrelated_types_visitor.dart 14:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// Base class for visitor used in rules where we want to lint about invoking
^^^
/src/util/unrelated_types_visitor.dart 110:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// Checks a [MethodInvocation] or [IndexExpression] which has a singular
  ^^^
/src/rules/package_api_docs.dart 110:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  ///  classMember ::=
  ^^^
/src/rules/package_api_docs.dart 119:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  ///  compilationUnitMember ::=
  ^^^
/src/rules/unnecessary_string_escapes.dart 55:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// The special escaped chars listed in language specification
  ^^^
/src/rules/use_super_parameters.dart 143:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// Check if all super positional parameters can be converted to use super-
  ^^^
/src/rules/invariant_booleans.dart 127:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// The only purpose of this rule is to report the second node on a contradictory
^^^
/src/rules/prefer_is_empty.dart 188:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// todo(pq): consider sharing
  ^^^
/src/rules/use_string_buffers.dart 49:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// The motivation of this rule is performance stuffs, and the explanation is
^^^
/src/rules/cascade_invocations.dart 162:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// This is necessary when you have a variable declaration so that element
  ^^^
/src/rules/prefer_single_quotes.dart 121:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// The only way to get immediate children in a unified, typesafe way, is to
^^^
/src/rules/prefer_single_quotes.dart 139:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// Do a top-down analysis to search for string nodes. Note, do not pass in
^^^
/src/rules/prefer_single_quotes.dart 146:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// Scan as little of the tree as possible, by bailing out on first match. For
  ^^^
/src/rules/prefer_final_parameters.dart 92:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// Report the lint for parameters in the [parameters] list that are not
  ^^^
/src/rules/control_flow_in_finally.dart 111:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// Do not extend this class, it is meant to be used from
^^^
/src/rules/use_enums.dart 97:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// A visitor used to visit the class being linted. This visitor throws an
^^^
/src/rules/use_enums.dart 135:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// A visitor that visits everything in the library other than the class being
^^^
/src/rules/null_closures.dart 199:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// Two [NonNullableFunction] objects are equal if their [library], [type],
  ^^^
/src/rules/avoid_web_libraries_in_flutter.dart 30:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// todo (pq): consider making a utility and sharing w/ `prefer_relative_imports`
^^^
/src/rules/prefer_mixin.dart 78:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// Check for "legacy" classes that cannot easily be made `mixin`s for
  ^^^
/src/formatter.dart 198:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// Override to influence error sorting
  ^^^
/src/ast.dart 32:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// Return the compilation unit of a node
^^^
/src/ast.dart 47:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// Returns the value of an [IntegerLiteral] or [PrefixExpression] with a
^^^
/src/ast.dart 66:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// If the [node] is the finishing identifier of an assignment, return its
^^^
/src/ast.dart 128:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// Return `true` if this compilation unit [node] is declared within a public
^^^
/src/ast.dart 392:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// If the [node] is the target of a [CompoundAssignmentExpression],
^^^
/src/cli.dart 47:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// todo (pq): consider using `dart analyze` where possible
^^^

238 files analyzed, 33 issues found, in 10833 ms.

Testing

Will write tests, if this looks worthy of merging.

Issues

Fixes dart-lang/sdk#57202
Fixes dart-lang/sdk#57907

coveralls · 2022-11-07T22:09:16Z

Coverage: 95.604% (-0.07%) from 95.671% when pulling ec3a0c0 on ditman:dartdoc_summary_line into dcf3a07 on dart-lang:main.

lib/src/rules/dartdoc_summary_line.dart

srawlins

This rule needs to account for elements which may follow a paragraph which are not necessarily preceded by blank lines. For example:

Paragraph.
* list without a blank

Paragraph.
```
code block without a blank
```

Paragraph.
> blockquote without a blank

Each of these paragraphs containing "Paragraph." should be viewed as a valid paragraph which contains one sentence ending in a period.

lib/src/rules/dartdoc_summary_line.dart

ditman · 2022-11-08T02:16:18Z

Each of these paragraphs containing "Paragraph." should be viewed as a valid paragraph which contains one sentence ending in a period.

@srawlins, some of those examples are flagged as invalid in my markdown linter, for example:

Paragraph.
* List element

Trips: MD032 - Lists should be surrounded by blank lines.

Paragraph.
```text
Code block
```

Trips: MD031 - Fenced code blocks should be surrounded by blank lines

(A blockquote immediately underneath the paragraph passes lint just fine!).

However, markdown validity aside, the way I interpret the dart recommendation is something like: the summary line "must be followed by a blank line", if there's any extra documentation after it.

Add a blank line after the first sentence to split it out into its own paragraph. If more than a single sentence of explanation is useful, put the rest in later paragraphs.

I think the cases you mentioned should fail, and the extra elements after the first line should not be treated in a special manner (even if our markdown parser is able to extract the first line correctly from them).

(PS: How much more breakage does this cause? :P)

srawlins · 2022-11-08T02:19:54Z

However, markdown validity aside, the way I interpret the dart recommendation is something like: the summary line "must be followed by a blank line", if there's any extra documentation after it.

That's fair.

ditman · 2022-11-08T03:05:54Z

@srawlins @pq one thing that I think I'm going to do is to remove the whole code branch about /**-doc comments. They seem to be discouraged in the style guide, and they add a bunch of complexity that I don't want to deal with.

What do you think? Any value in keeping the doc generation from /** comments shiny?

srawlins · 2022-11-08T03:09:35Z

What do you think? Any value in keeping the doc generation from /** comments shiny?

No, I think they could be ignored completely.

ditman · 2022-11-08T04:05:57Z

linter / main seems to be failing because I modified lib/src/rules.dart, but never updated the test that asserts the list of lints, I'll fix that as part of adding unit tests to this PR.

ditman · 2022-11-08T05:00:44Z

After the changes, the linter is a little bit more "noisy", because it marks more tokens than before (especially when the comment is "too long"), but the errors themselves are pretty much the same:

dit@dit:~/github/dart-linter$ dart run bin/linter.dart --rules=dartdoc_summary_line ~/github/dart-linter/lib

/usr/local/google/home/dit/github/dart-linter/lib/src/util/score_utils.dart 11:48 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// todo(pq): de-duplicate these fetches / URIs
                                               ^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/tested_expressions.dart 126:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// assuming properties are pure computations. i.e. dealing with De Morgan's
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/tested_expressions.dart 126:48 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// assuming properties are pure computations. i.e. dealing with De Morgan's
                                               ^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/tested_expressions.dart 127:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// laws https://en.wikipedia.org/wiki/De_Morgan%27s_laws
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/tested_expressions.dart 127:60 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// laws https://en.wikipedia.org/wiki/De_Morgan%27s_laws
                                                           ^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/flutter_utils.dart 57:56 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// See: analysis_server/lib/src/utilities/flutter.dart
                                                       ^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/ascii_utils.dart 5:78 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// String utilities that use underlying ASCII codes for improved performance.
                                                                             ^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/ascii_utils.dart 7:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// loop would do (and would do so far more performantly).
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/ascii_utils.dart 8:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// See: https://github.com/dart-lang/linter/issues/1828
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/ascii_utils.dart 8:57 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// See: https://github.com/dart-lang/linter/issues/1828
                                                        ^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/condition_scope_visitor.dart 133:76 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// todo (pq): here and w/ getTrueExpressions, consider an empty iterable
                                                                           ^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/leak_detector_visitor.dart 42:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// from building (predicates) with the provided [predicateBuilders] evaluated
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/leak_detector_visitor.dart 43:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// in the variable.
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/unrelated_types_visitor.dart 16:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// unrelated to the singular type argument of the class. Extending this
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/unrelated_types_visitor.dart 16:57 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// unrelated to the singular type argument of the class. Extending this
                                                        ^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/unrelated_types_visitor.dart 17:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// visitor is as simple as knowing the methods, classes and libraries that
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/unrelated_types_visitor.dart 18:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// uniquely define the target, i.e. implement only [methods].
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/unrelated_types_visitor.dart 112:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// type of [collectionType].
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/version.dart 5:20 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// Package version.  Synchronized w/ pubspec.yaml.
                   ^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/package_api_docs.dart 112:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  ///  | [FieldDeclaration]
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/package_api_docs.dart 113:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  ///  | [MethodDeclaration]
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/package_api_docs.dart 113:29 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  ///  | [MethodDeclaration]
                            ^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/package_api_docs.dart 121:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  ///  | [EnumDeclaration]
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/package_api_docs.dart 122:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  ///  | [FunctionDeclaration]
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/package_api_docs.dart 123:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  ///  | [TopLevelVariableDeclaration]
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/package_api_docs.dart 124:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  ///  | [ClassTypeAlias]
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/package_api_docs.dart 125:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  ///  | [FunctionTypeAlias]
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/package_api_docs.dart 125:29 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  ///  | [FunctionTypeAlias]
                            ^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/unnecessary_string_escapes.dart 55:65 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// The special escaped chars listed in language specification
                                                                ^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/use_super_parameters.dart 144:19 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// initializers. Return a list of convertible named parameters or `null` if
                  ^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/use_super_parameters.dart 145:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// there are parameters that can't be converted since this will short-circuit
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/use_super_parameters.dart 146:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// the lint.
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/dartdoc_summary_line.dart 111:47 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// however 'brief' is not defined perfectly. Instead of enforcing a fixed
                                              ^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/dartdoc_summary_line.dart 112:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// number of characters, we just look at how many lines are part of the
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/dartdoc_summary_line.dart 113:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// summary, and if we deem them to be too many (2 should be plenty), we fail
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/dartdoc_summary_line.dart 114:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// the lint
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/dartdoc_summary_line.dart 114:15 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// the lint
              ^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/prefer_is_empty.dart 188:33 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// todo(pq): consider sharing
                                ^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/use_string_buffers.dart 51:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// algorithm is O(~N^2) and that is because a String is not mutable, so in each
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/use_string_buffers.dart 52:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// step it creates an auxiliary String that takes O(amount of chars) to be
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/use_string_buffers.dart 53:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// computed, in otherwise using a StringBuffer the order is reduced to O(~N)
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/use_string_buffers.dart 54:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// so the bad case is N times slower than the good case.
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/cascade_invocations.dart 164:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// in the right part of an assignment in a following expression that we would
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/cascade_invocations.dart 165:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// like to join to this.
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/prefer_single_quotes.dart 74:60 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// Strings interpolations can contain other string nodes. Check like this.
                                                           ^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/prefer_single_quotes.dart 81:56 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// Strings can be within interpolations (ie, nested). Check like this.
                                                       ^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/prefer_single_quotes.dart 122:59 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// call visitChildren on that node, and pass in a visitor. This collects at the
                                                          ^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/prefer_single_quotes.dart 123:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// top level and stops.
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/prefer_single_quotes.dart 139:54 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// Do a top-down analysis to search for string nodes. Note, do not pass in
                                                     ^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/prefer_single_quotes.dart 141:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// its children.
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/prefer_single_quotes.dart 146:76 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// Scan as little of the tree as possible, by bailing out on first match. For
                                                                           ^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/prefer_single_quotes.dart 148:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// true, or they will return false because leaves have no children.
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/control_flow_in_finally.dart 113:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// configurability given that reporting throw statements in a finally clause is
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/control_flow_in_finally.dart 114:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// controversial.
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/use_enums.dart 97:51 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// A visitor used to visit the class being linted. This visitor throws an
                                                  ^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/use_enums.dart 99:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// top-level expression of an initializer for one of the fields being converted.
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/use_enums.dart 136:11 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// linted. This visitor throws an exception if
          ^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/use_enums.dart 137:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// - there is a subclass of the class, or
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/use_enums.dart 138:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// - there is an invocation of one of the constructors of the class.
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/null_closures.dart 201:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// invocation is among a collection of non-nullable functions.
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/avoid_web_libraries_in_flutter.dart 30:82 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// todo (pq): consider making a utility and sharing w/ `prefer_relative_imports`
                                                                                 ^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/prefer_mixin.dart 79:28 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// compatibility reasons.
                           ^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/prefer_mixin.dart 80:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// (See: https://github.com/dart-lang/linter/issues/2082)
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/rules/prefer_mixin.dart 80:61 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// (See: https://github.com/dart-lang/linter/issues/2082)
                                                            ^
/usr/local/google/home/dit/github/dart-linter/lib/src/formatter.dart 198:42 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// Override to influence error sorting
                                         ^
/usr/local/google/home/dit/github/dart-linter/lib/src/ast.dart 32:42 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// Return the compilation unit of a node
                                         ^
/usr/local/google/home/dit/github/dart-linter/lib/src/ast.dart 48:39 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// minus and then an [IntegerLiteral]. If a [context] is provided,
                                      ^
/usr/local/google/home/dit/github/dart-linter/lib/src/ast.dart 49:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// [SimpleIdentifier]s are evaluated as constants. For anything else,
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/ast.dart 50:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// returns `null`.
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/ast.dart 68:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// thought as the "readElement".
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/ast.dart 129:54 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// directory in the given [package]'s directory tree. Public dirs are the
                                                     ^
/usr/local/google/home/dit/github/dart-linter/lib/src/ast.dart 130:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// `lib` and `bin` dirs.
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/ast.dart 270:63 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// Uses [processor] to visit all of the children of [element].
                                                              ^
/usr/local/google/home/dit/github/dart-linter/lib/src/ast.dart 394:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// the setter referenced with a [SimpleIdentifier] or a [PropertyAccess],
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/ast.dart 395:1 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// or the `[]=` operator.
^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/ast.dart 423:41 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// An [Element] processor function type.
                                        ^
/usr/local/google/home/dit/github/dart-linter/lib/src/cli.dart 48:55 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
/// see: https://github.com/dart-lang/linter/pull/2537
                                                      ^

238 files analyzed, 77 issues found, in 10938 ms.

A single comment can trigger the linter several times, for example the following comment triggers the linter 4 times:

https://github.com/dart-lang/linter/blob/657fa6d60373c144777ddb3ca0ab6ee7805ea2b3/lib/src/util/tested_expressions.dart#L124-L128

/usr/local/google/home/dit/github/dart-linter/lib/src/util/tested_expressions.dart 126:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// assuming properties are pure computations. i.e. dealing with De Morgan's
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/tested_expressions.dart 126:48 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// assuming properties are pure computations. i.e. dealing with De Morgan's
                                               ^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/tested_expressions.dart 127:3 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// laws https://en.wikipedia.org/wiki/De_Morgan%27s_laws
  ^^^
/usr/local/google/home/dit/github/dart-linter/lib/src/util/tested_expressions.dart 127:60 [lint] Start your DartDoc comment with a single, brief sentence that ends with a period.
  /// laws https://en.wikipedia.org/wiki/De_Morgan%27s_laws
                                                           ^

Problems with the comment:

takes too many lines (not brief) (126:3, 127:3)
does not end in a period (127:60)
seemingly contains more than one sentence (126:48)

Parsed output, per file in the repo:

     11 /usr/local/google/home/dit/github/dart-linter/lib/src/ast.dart
      9 /usr/local/google/home/dit/github/dart-linter/lib/src/rules/package_api_docs.dart
      8 /usr/local/google/home/dit/github/dart-linter/lib/src/rules/prefer_single_quotes.dart
      5 /usr/local/google/home/dit/github/dart-linter/lib/src/util/unrelated_types_visitor.dart
      5 /usr/local/google/home/dit/github/dart-linter/lib/src/rules/use_enums.dart
      5 /usr/local/google/home/dit/github/dart-linter/lib/src/rules/dartdoc_summary_line.dart
      4 /usr/local/google/home/dit/github/dart-linter/lib/src/util/tested_expressions.dart
      4 /usr/local/google/home/dit/github/dart-linter/lib/src/util/ascii_utils.dart
      4 /usr/local/google/home/dit/github/dart-linter/lib/src/rules/use_string_buffers.dart
      3 /usr/local/google/home/dit/github/dart-linter/lib/src/rules/use_super_parameters.dart
      3 /usr/local/google/home/dit/github/dart-linter/lib/src/rules/prefer_mixin.dart
      2 /usr/local/google/home/dit/github/dart-linter/lib/src/util/leak_detector_visitor.dart
      2 /usr/local/google/home/dit/github/dart-linter/lib/src/rules/control_flow_in_finally.dart
      2 /usr/local/google/home/dit/github/dart-linter/lib/src/rules/cascade_invocations.dart
      1 /usr/local/google/home/dit/github/dart-linter/lib/src/version.dart
      1 /usr/local/google/home/dit/github/dart-linter/lib/src/util/score_utils.dart
      1 /usr/local/google/home/dit/github/dart-linter/lib/src/util/flutter_utils.dart
      1 /usr/local/google/home/dit/github/dart-linter/lib/src/util/condition_scope_visitor.dart
      1 /usr/local/google/home/dit/github/dart-linter/lib/src/rules/unnecessary_string_escapes.dart
      1 /usr/local/google/home/dit/github/dart-linter/lib/src/rules/prefer_is_empty.dart
      1 /usr/local/google/home/dit/github/dart-linter/lib/src/rules/null_closures.dart
      1 /usr/local/google/home/dit/github/dart-linter/lib/src/rules/avoid_web_libraries_in_flutter.dart
      1 /usr/local/google/home/dit/github/dart-linter/lib/src/formatter.dart
      1 /usr/local/google/home/dit/github/dart-linter/lib/src/cli.dart

ditman · 2022-11-08T05:12:06Z

(Seems that I can only re-request review from one of the reviewers, odd)

srawlins · 2022-11-08T05:11:12Z

lib/src/rules/dartdoc_summary_line.dart

+  /// The summary line should be 'brief' according to the Dart recommendations,
+  /// however 'brief' is not defined perfectly. Instead of enforcing a fixed
+  /// number of characters, we just look at how many lines are part of the
+  /// summary, and if we deem them to be too many (2 should be plenty), we fail


I think we need some data on this point. If plenty of "fine" doc comments exist which are written across more than 2 lines, we'll have to adjust, so that the rule can be used.

Since part of this is geared towards the generated Dartdoc... Do we have examples where the generated dartdoc looks good/bad because of the summary, and try to infer what's a good limit here? IIRC Dartdoc trimmed this at some point? (or I remember a [...] which pointed to the full doc page or something?)

If unlimited, people can write walls of text as the "first line" of the dartdoc, and that kind of defeats the purpose of the lint I think :/

Drive by: Given that it's important for the lint and for dartdoc to agree, have we considered moving the logic that dartdoc uses to find the summary into the markdown package (or maybe even to the analyzer package as a getter on Comment) so that it can be shared? Having the logic in two places opens the door for them becoming inconsistent in the future.

dartdoc takes the inner HTML of the first HTML node of the rendered documentation. We could move this logic to be shared, but it requires dartdoc's custom resolution logic, so we'd have to move that too.

https://github.com/dart-lang/dartdoc/blob/master/lib/src/render/documentation_renderer.dart#L61

So how does it decide what to include in the first HTML node?

It parses the documentation comment and renders it into HTML. So the user determines what is in the first HTML node.

In this markdown:

header === text

the first HTML node is <h1>header</h1>, whose inner HTML is header. If the first HTML node is a <p> node with a gigabyte of HTML inside, then that is the first node, and the gigabyte of HTML is displayed.

Right. I should probably drop out of the conversation with apologies for distracting everyone. It just seemed to me that the logic dartdoc / markdown uses to determine what's in the first HTML node could be used without needing to first build the HTML and then pull it back out. But if it isn't worth the effort then it isn't.

srawlins · 2022-11-08T05:12:24Z

lib/src/rules/dartdoc_summary_line.dart

+
+    if (!_endsWithPeriod(summary)) {
+      Token last = summary.last;
+      // Highlight the last character of the sentence...


Just a comment for when this comes out of WIP: usually 1 character is too short; it is too hard to see the squiggles in the IDE.

srawlins · 2022-11-08T05:13:34Z

lib/src/rules/dartdoc_summary_line.dart

+String _trimSlashes(String line) => line.replaceFirst('///', '');
+
+/// Determines if a token `tk` represents an empty comment line.
+bool _isNotBlankLine(Token tk) => _trimSlashes(tk.lexeme).trim().isNotEmpty;


Nit for when not wip: can call it token, to be more idiomatic. https://dart.dev/guides/language/effective-dart/documentation#avoid-abbreviations-and-acronyms-unless-they-are-obvious

srawlins · 2022-11-08T05:26:50Z

lib/src/rules/dartdoc_summary_line.dart

+    node.tokens.takeWhile(_isNotBlankLine);
+
+/// Checks that the last of the `lines` ends in a period.
+bool _endsWithPeriod(Iterable<Token> tokens) =>


I think we'll need to allow colons as well. For example: https://dart.dev/guides/language/effective-dart/documentation#prefer-backtick-fences-for-code-blocks

pq · 2022-11-08T17:39:25Z

Any value in keeping the doc generation from /** comments shiny?

Nope. I'm all for ignoring them.

pq · 2022-11-08T18:00:29Z

A single comment can trigger the linter several times

I'm sort of inclined to just report one thing ("malformed comment bad") and consider dropping the details on the floor. With a pointer to a doc, folks can likely figure out what's wrong. Alternatively, if you want to stay specific, just report the first one and expect folks to iterate.

pq · 2022-11-08T18:08:20Z

lib/src/rules/dartdoc_summary_line.dart

+      // Highlight the last character of the sentence...
+      rule.reportLintForOffset(last.offset + last.lexeme.length, 1);
+    }
+    int? sentenceBreakOffset = _findSentenceBreak(summary);


The rule description says that the summary should only be one sentence:

start doc comments with a single-sentence summary.

and if we just enforced that things would get a lot simpler...

Was that just too limiting in practice?

Having more than one sentence is probably myself contradicting my own idea of "just because the markdown parser can handle it, it doesn't mean it's good"; so yeah, maybe just enforcing a literal one line as the summary simplifies everything a bunch. Then the length of that line is (maybe) taken care of by another lint rule.

ditman · 2022-11-08T22:22:43Z

I've converted this to a draft, I'll iterate over this on the weekend again, and will possibly start adding unit tests to this.

… summary is too long.

ditman · 2023-02-18T03:29:25Z

Reviving this in my new cloudtop so I can work on it again. I don't remember anything though :P

mosuem · 2024-03-12T11:51:25Z

@ditman is this still WIP?

Closing this for now. Feel free to reopen in case you still want to land it!

ditman · 2024-03-12T22:41:20Z

@mosuem I want to land this, but I'm not sure if I want to land it as a "native" lint, or something from package:custom_lint?

ditman changed the title ~~[wip] Dartdoc Summary Line lint~~ [wip] Introduce DartDoc "Summary Line" lint Nov 7, 2022

pq reviewed Nov 7, 2022

View reviewed changes

lib/src/rules/dartdoc_summary_line.dart Outdated Show resolved Hide resolved

lib/src/rules/dartdoc_summary_line.dart Outdated Show resolved Hide resolved

lib/src/rules/dartdoc_summary_line.dart Outdated Show resolved Hide resolved

srawlins reviewed Nov 7, 2022

View reviewed changes

ditman requested review from pq and srawlins and removed request for pq and srawlins November 8, 2022 05:09

srawlins reviewed Nov 8, 2022

View reviewed changes

pq reviewed Nov 8, 2022

View reviewed changes

ditman marked this pull request as draft November 8, 2022 22:21

palisadoes mentioned this pull request Feb 10, 2023

Create a documentation comment linter that is compatible with DartDoc PalisadoesFoundation/talawa#1479

Closed

ditman added 5 commits February 17, 2023 18:59

Adds dartdoc_summary_line lint

9c5393a

Wires the new lint into the CLI.

e6dc84f

Remove slash-star-star code path. Address PR comments.

07b1ed5

Mark the beginning of the comment rather than the whole line when the…

799cc59

… summary is too long.

Locate separate sentences in different extracted tokens.

ec3a0c0

ditman force-pushed the dartdoc_summary_line branch from 56625dd to ec3a0c0 Compare February 18, 2023 03:02

Simplify things a lot. Do not trip on periods in the middle of the line.

f14d462

ditman mentioned this pull request May 30, 2023

[google_maps] Prepares packages to endorse web. flutter/packages#4064

Merged

11 tasks

mosuem closed this Mar 12, 2024

[wip] Introduce DartDoc "Summary Line" lint #3820

[wip] Introduce DartDoc "Summary Line" lint #3820

Uh oh!

Conversation

ditman commented Nov 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

OK:

OK:

BAD:

BAD:

BAD:

IGNORED:

Manual testing

Testing

Issues

Uh oh!

coveralls commented Nov 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

srawlins left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ditman commented Nov 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

srawlins commented Nov 8, 2022

Uh oh!

ditman commented Nov 8, 2022

Uh oh!

srawlins commented Nov 8, 2022

Uh oh!

ditman commented Nov 8, 2022

Uh oh!

ditman commented Nov 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ditman commented Nov 8, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srawlins Nov 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pq commented Nov 8, 2022

Uh oh!

pq commented Nov 8, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ditman commented Nov 8, 2022

Uh oh!

ditman commented Feb 18, 2023

Uh oh!

mosuem commented Mar 12, 2024

ditman commented Nov 7, 2022 •

edited

Loading

coveralls commented Nov 7, 2022 •

edited

Loading

ditman commented Nov 8, 2022 •

edited

Loading

ditman commented Nov 8, 2022 •

edited

Loading

srawlins Nov 8, 2022 •

edited

Loading