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

Introduce a syntax for referencing out-of-scope elements from a doc comment. #49073

Closed
srawlins opened this issue May 20, 2022 · 12 comments
Closed
Assignees
Labels
analyzer-ux area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@srawlins
Copy link
Member

srawlins commented May 20, 2022

Introduce a syntax for referencing out-of-scope elements from a doc comment.

WHAT PROBLEM IS THIS SOLVING?

Users can reference elements in doc comments (///-style) with square brackets. For example, /// Returns a [Gadget] and /// Must be called after [Gadget.initialize]. These elements can then be cross-linked in the IDE, code search, and doc sites.

However, such elements must be in-scope; they must be imported into, or declared within, the library with the doc comment. This design introduces a syntax for referencing out-of-scope elements.

BACKGROUND

Audience

Anyone writing documented Dart code.

Glossary

  • Doc comment (or dartdoc comment or documentation comment) - a Dart code comment in which each line starts with three slashes (///), and which is placed just before a library element (class, top-level function, top-level constant, etc.) or a class/mixin/enum/extension element (method, field, etc.). A doc comment is parsed (by various tools) as Markdown text.
  • Doc comment reference - the name of any element wrapped in square brackets, inside a doc comment. This is typically one identifier or two separated by a period, like [List] or [Future.wait].
  • Doc site - the website generated by the dartdoc tool for a package.
  • In-scope element - a Dart element (like a class, top-level function, top-level constant, field, method, etc.) which is either imported into a library or declared in a library is in-scope in that library.
  • Dart URI resolution - the strategy for resolving a URI string into a path in order to locate a Dart library (or part) file.
  • A Dart library’s export namespace - the collection of Dart elements exported by a library, which includes all public library elements declared in a library (including parts) and all elements exported by the export directives in the library (taking into account any show and hide combinators).

OVERVIEW

The basic design is to allow out-of-scope elements to be referenced in a doc comment by referencing the element’s library, via the same support URIs, and name, delimited by a hash mark (#). Quick examples:

/// See [dart:html#Element].
/// See [package:flutter/element.dart#Element].
/// See [../path/to/somewehere.dart#Element].
/// See [package:flutter/widgets.dart].

That last example indicates that a reference can also be made to a Dart library.

Non-goals

There are several asks to enhance the support of what can go into a doc comment. For example indicating a callable element with parentheses (e.g. /// [Future.wait()]) or multiple properties (e.g. /// [myList.toSet().length]) or even just snippets of code (e.g. /// [await thisMethod()] or /// [thisConst + thatConst]). These also just show up as plain, unlinked text in the IDE and in doc sites. Enhancements like these are out-of-scope.

USAGE EXAMPLES

Examples of legal usage of the new syntax:

/// See [dart:html#Element].

/// See [package:flutter/element.dart#Element].

/// See [../path/to/somewehere.dart#Element].

/// See [dart:core#Future] which is declared in dart:async.

/// See [dart:async#Future.wait].

/// See [dart:async#Future.value].

/// See [package:flutter/widgets.dart].

Examples of non-working usage of the new syntax:

/// See [package:foo/foo.dart#_PrivateClass].

/// See [package:foo/foo.dart#Foo._privateField].

/// See [package:foo/foo.dart#Foo._privateConstructor].

/// See [package:foo/foo.dart#_PrivateClass.publicMethod].

/// See [package:not_found_in_pubspec/foo.dart#Foo].

DETAILED DESIGN/DISCUSSION

When a tool which parses doc comment references sees a possible element reference between square brackets, it can look for a hash mark (#), and resolve the text to the left as a URI, and the right as an element (one identifier or two separated by a period). Such tools have all of the know-how to parse import URIs, and collect the namespace exported by a library at the resolved URI location. This means that an element can be located by the library in which it is declared, or by any library which exports the element.

The import URI syntax is chosen for several simple reasons:

  • It allows referring to an element by exactly where it is declared (or exported).
  • It is familiar.
  • It allows referring to elements which are in a different package, in a Dart SDK library, or in a relative library.
  • It allows referring to libraries directly. An IDE can support jumping to a library file, and dartdoc can support linking to a library page (if it is "public").

There is an interesting consequence of using the import URI syntax and the exact URI parsing mechanism: In order for the IDE to cross-link references with a package: schema, the indicated package must be "known" in the current package’s package configuration (e.g. pubspec). It is conceivable that cross-package links could be made in doc sites even if the linked package is not a known by the package’s configuration, but this behavior should not be expected or relied on.

Requiring a referenced package to be known in the referring package’s package configuration could be seen as detrimental. But in the common case (the "pub package" case), it is not expensive to add a referenced package to the dev_dependencies, and this solid link will allow static analysis tools to continue to report when a doc comment reference becomes "broken."

Using the export namespace of the library at the resolved URI location has the following consequences:

  • An element which is declared in a "implementation-private" library (in a src/ directory) does not have to be referred to by that library’s URI; it can be referred to by whatever "public" library exports it.
  • A private element (e.g. a class named _Foo or a field named _foo) cannot be referred to, outside of the library in which it is declared. I consider this a feature, but if there is strong desire to be able to refer to such elements, we could change the mechanics from "a library’s export namespace" to something made up.

ACCESSIBILITY

I am not aware of any accessibility considerations.

INTERNATIONALIZATION

I am not aware of any internationalization or localization considerations.

OPEN QUESTIONS

Is the hash mark a bad delimiter? I chose it because it looks like an HTML document fragment delimiter. I haven’t found any downsides to it, and haven’t seen a better delimiter to use.

TESTING PLAN

Support for this syntax will be tested in:

  • the parser’s unit tests
  • analyzer’s unit tests
  • analysis server’s navigation tests, hover tests, etc.
  • dartdoc’s reference unit tests

DOCUMENTATION PLAN

I am not aware of any documentation for how to write and format doc comments outside of Effective Dart. Effective Dart does have an entry encouraging using square brackets to reference in-scope identifiers, which will be updated. If there is a need for a guide, an article, or other documentation for doc comments, beyond what is provided in Effective Dart, that is out-of-scope for this design.

MIGRATION PLAN

If a developer has wanted to reference an out-of-scope element, there are a few strategies they may have ultimately chosen:

  • Use backticks instead. For example, /// See `List`. For this case, we can offer a lint and a quick fix, "this could be a doc comment reference." (This could have many false positives.)
  • Import the element, making it now in-scope, just to be able to reference it. In this case, we could report an "import only used for comment references" lint, with a fix which removes the import and changes each comment reference to use the out-of-scope syntax.
  • Ignore the fact that the IDE won't cross-link, rely on dartdoc's ability to find far-flung elements. For this case, dartdoc will start reporting a warning that such an element will not be discoverable with the current, ambiguous, syntax, and will suggest replacement text. A shell script which parses such warnings could act as a migration tool.

This addresses #44637.

@srawlins srawlins added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request analyzer-ux type-enhancement A request for a change that isn't a bug labels May 20, 2022
@srawlins srawlins changed the title Syntax for dartdoc references to not-imported elements Introduce a syntax for referencing out-of-scope elements from a doc comment. Jul 7, 2022
@srawlins srawlins self-assigned this Jul 13, 2022
@kevmoo
Copy link
Member

kevmoo commented Jul 19, 2022

Only comment: wondering if we should "lint" relative paths that point from public APIs to non-public APIs.

Like a link in lib/foo.dart to [../test/my_test.dart#Bar] would be bad – not resolvable.

But a link within the the test/ directory may be nice for code navigation 🤷

@goderbauer
Copy link
Contributor

Do you have any data on how common the current strategies for linking to out-of-scope elements mentioned in the "MIGRATION PLAN" section are in a larger codebase like, say, Flutter? I am wondering how much work it would be to make Flutter's docs warning-free again after this proposal has been implemented...

@kevmoo
Copy link
Member

kevmoo commented Jul 19, 2022

To pile on: it would be GREAT to have tooling to flag imports that only exist to support doc comment references.

To pile on more: add a quick-fix to replace these w/ the new syntax 🤯

@srawlins
Copy link
Member Author

srawlins commented Jul 19, 2022

@kevmoo those ideas are in the migration plan. Great minds... 😄

@goderbauer I've found in flutter stats for "what if we didn't count a doc comment reference as a usage? How many imports would become "unused"? There are about ~50 imports like this in flutter/flutter, and ~10 in flutter/plugins.

I haven't yet figured out how many cases there are of /// See [Element] where Element is out of scope. I'm working on it... but dartdoc's custom resolution is a dark and mysterious place...

@goderbauer
Copy link
Contributor

From design review:

  • What about linking to elements in other packages that the given package can't depend on (e.g. link from dart:ui into package:flutter)?
  • How will this syntax render in HTML?

@parlough
Copy link
Member

You mentioned a documentation plan is out of scope of this design, but it may be worth include adding some information to Dartdoc comment references once implemented.

If dartdoc is committing to this reference syntax, it might be time to more formally specify the entire syntax on a dedicated page on dart.dev as well :)

@parlough
Copy link
Member

parlough commented Jul 19, 2022

As for how it will render in HTML, it would be nice to only include the final element (after the # in those cases) if there are no conflicts and perhaps show the fully specified reference (likely always as a global reference, not a local/relative reference) on hover.

IDEs may be able to implement similar logic to compress the longer references visually (then expand on click or something) and perhaps other tools but this wouldn't really handle the long case on GitHub.

@srawlins
Copy link
Member Author

srawlins commented Jul 19, 2022

Thanks @goderbauer and @parlough!

More notes:

  • The number of references in Flutter to far-flung elements is a rather crucial consideration. On one test I've run, the number might be as high as 13,000 references 😮‍💨 . Changing 13,000 cases of [Element] to [package:flutter/widgets.dart#Element] may make doc comments substantially harder to read in the IDE or GitHub.

    Edit: I think 11,500 of these cases are found in inherited documentation, and is therefore duplicate (yes?).

  • Another feature was suggested, in which a package (or library?) may declare its "doc scope." For example, we could allow declaring a "doc scope" in dartdoc_options.yaml, or perhaps a doc_dependencies section in pubspec.yaml (would this solve the dart:ui case?)

@lrhn
Copy link
Member

lrhn commented Jul 19, 2022

Another alternative, a little more low-tech, would be to have doc-only imports. Say a comment like:

/// @dartdoc import "package:flutter/widget.dart"

would make the contents of that library available for DartDoc referencing, as if it had been imported normally (probably taking precedence in case of a conflict, since it's intended for documenting.

It means package:flutter needs to be at least a dev-dependency for the package containing that doc-import.

@gspencergoog
Copy link
Contributor

FYI, I've transcribed this proposal into a design doc that is shared publicly for easier commenting.

@srawlins
Copy link
Member Author

I've written an alternative proposal for an import syntax, at #50702, based on @lrhn 's comment above.

@srawlins
Copy link
Member Author

We're going with #50702. CC @kallentu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-ux area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. 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

6 participants