Introduce a syntax for importing elements just for doc comment references

[srawlins]

This is an alternative proposal to https://github.com/dart-lang/sdk/issues/49073. This proposal improves upon the readability and perhaps the simplicity of that proposal. It is based on @lrhn 's brief comment on that proposal.

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].
• Top-level namespace - a set of top-level elements defined by one or more import directives and the current library, which share a common prefix. A library has a set of one or more top-level namespaces.
• Un-prefixed namespace - the unambiguous union of all elements provided by all of the un-prefixed import directives (no name collisions of non-identical elements are allowed) of a library, plus all elements declared in the library (which may shadow any imported elements).
• Prefixed namespace - the unambiguous union of all elements provided by all of the import directives which share a common (non-empty) prefix (no name collisions of non-identical elements are allowed).
• 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 add a new annotation, perhaps to the meta package, @docImport which looks similar to an import directive. When a library is annotated with this annotation, the analyzer will add elements to one of a library's top-level namespace, just like an import directive. Doc comment references can then resolve to elements provided by such "doc imports". Quick examples:

@docImport('dart:async')
@docImport('package:flutter/element.dart', show: ['Element'])
@docImport('../path/to/somwhere.dart')
@docImport('dart:html', as: 'html')
library;

/// We can now reference [Future] from dart:async, [Element] from Flutter's element library,
/// and [html.Element] from dart:html, even if none of these libraries are actually imported
/// by this library.
class Foo {}

DETAILED DESIGN/DISCUSSION

A class is introduced which can be used as an annotation:

class docImport {
  final String uri;
  final String? prefix;
  final Set<String> shownNames;
  final Set<String> hiddenNames;
  const docImport(this.uri, {
    String? as,
    Set<String> show,
    Set<String> hide,
  }) :
      this.prefix = as,
      this.shownNames = show
      this.hiddenNames = hide;
}

Each docImport annotating a library directive introduces a doc import which contributes to a new doc namespace.

A library currently has one or more top-level namespaces: the un-prefixed namespace and zero or more prefixed namespaces, which result from import directive(s) with prefixes.

Each identifier in a library (both in code and in doc comment references) is resolved according to various scoping rules, which may resolve to a non-top-level identifier, or a top-level identifier from one of the top-level namespaces. (This is a simplification since a prefix itself is an identifier which needs resolution, and which can also be shadowed by a non-top-level identifier.)

This change introduces a second set of namespaces used only for the resolution of doc comments: the "doc namespaces." The doc namespaces are a superset of the regular namespaces, and each doc namespace is a superset of the corresponding regular namespace (with the same prefix).

For a given library, the doc namespace with prefix p is computed via the following steps:

1. Start with an empty set of elements.
2. For each import with prefix p:
   1. For each element provided by the import, according to the rules of 'show' and 'hide':
      • If there is an element with the same name already in the namespace, report a compile-time error.
      • Otherwise, add the element to the namespace.
3. For each @docImport annotation specified with prefix p:
   1. For each element provided by the import, according to the rules of 'show' and 'hide':
      • If there is an element with the same name already in the namespace, report a static warning.
      • Otherwise, add the element to the namespace.
4. If this is the un-prefixed doc namespace, then for each top-level element declared in the library:
   1. Add the element to the namespace, replacing any element currently in the namespace with the same name (shadowing it).

(The "regular" namespaces are computed in the exact same way, excluding step 3.)

Each doc comment reference is then resolved using the standard scoping rules, and using the doc namespaces as the set of top-level namespaces.

New static warnings are reported in the following situations:

• when the URI cannot be resolved, according to the URI-resolving rules
• when elements from a doc import have name conflicts with each other, or elements from imports with the same prefix
• when a prefix name does not match certain identifier rules
• when a show or hide clause includes names of elements which are not found in the library's export namespace.
• when a doc import is redundant, duplicate, or unnecessary (various subtle definitions) with a regular import or another doc import (lower priority)
• when a doc import is unused (lower priority)
• when an element in a show or hide clause of a doc import is unused (lower priority)

ACCESSIBILITY

I am not aware of any accessibility considerations.

INTERNATIONALIZATION

I am not aware of any internationalization considerations.

OPEN QUESTIONS

1. Can we define the docImport class and constructor in dart:core? This would be required if we want to use docImport functionality in the Flutter engine, which cannot import package:meta.

2. If not, should we instead support this feature as some sort of directive in a doc comment? For example, instead of the annotations in the OVERVIEW section above, we could instead support:

   /// @doc import 'dart:async';
   /// @doc import 'package:flutter/element.dart' show Element;
   /// @doc import '../path/to/somwhere.dart';
   /// @doc import 'dart:html' as html;
   library;

   This solution has some significant drawbacks:

   • It would be much more complicated to parse all of this syntax in doc comments, making it error-prone.
   • For all of the error cases of the parsing, we'd have to report errors, as long as it really looked like a doc import was intended.
   • We wouldn't get any syntax highlighting for free. That would also have to be implemented, and implemented once for each syntax highlighter we try to keep up (DAS, highlight.js, CodeMirror, Kythe, ...)

TESTING PLAN

Support for this syntax will be tested in:

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

DOCUMENTATION PLAN

The support for this feature can be documented on the docImport class.

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 adds a corresponding doc import.
• 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 a doc import. A shell script which parses such warnings could act as a migration tool.

This addresses https://github.com/dart-lang/sdk/issues/44637.

[lrhn]

Can we define the docImport class and constructor in dart:core.

We obviously can, but I'd prefer not to.

We generally try to avoid having tool-specific annotations (or annotations at all, since all annotations are tool specific to some extent) in the platform libraries. Platform libraries are prime real-estate. Adding annotations that makes sense for one tool initially seems reasonable, but when multiple tools starts doing it, it doesn't scale.

Changes to the annotations are driven by the tools, not the library maintainers, which splits the responsibility for maintaining platform libraries into multiple teams.

Because platform libraries are hard to change, it also locks the tools into the current implementation. Changing almost anything becomes a breaking change, which can dampen the velocity of tool development. The tool would be locked to SDK minor-version releases for adding any new features, and major versions (or the breaking-change process) for removing or breaking anything.

A tool can easily support multiple versions of their own metadata at the same time, but the platform libraries would loathe to have multiple versions, some probably deprecated, at the same time. Going through a multi-version deprecation, breaking-change and migration process in order to change an annotation isn't really worth it.

We have pragma as the compromise. It's for our own tools, or confident expert uses, to make notes on source code without needing to add visible annotations types to the public namespaces. It's not really for normal end users. A @pragma("dartdoc:import", "package:foo/foo.dart as p show Foo") can work, but it's not convenient.

(Using a package import to support dartDoc is clearly not viable. It would be adding an import to an otherwise unnecessary library, in order to avoid writing imports to otherwise unnecessary libraries.)

I would recommend using // @doc .... import syntax, and work with the analyzer on recognizing them for highlighting and possibly linking (if that's who controls that, plain regexp-based high-lighting seems doable too).

If enough people think the annotation will be highly valuable to many users, expected to be stable enough to not require frequent updates, and otherwise generally worth it, it's definitely technically possible to add it to dart:core.

We have added other things that are tool-related (like unawaited which only exists because we chose to add the related lint to our recommended set).

[srawlins]

cc @gspencergoog @bwilkerson

If enough people think the annotation will be highly valuable to many users, expected to be stable enough to not require frequent updates, and otherwise generally worth it, it's definitely technically possible to add it to dart:core.

I imagine it being used almost never. It's really just for the flutter/engine and flutter/flutter repos. And some users here and there will use it, but I think an order of magnitude less than unawaited.

That leaves us with:

• @docImport declared in package:meta and I think it would also have to be declared in dart:ui, so that flutter/engine could use the annotation. The analyzer would have to recognize annotations from both libraries.

• /// @doc import, which I really prefer not to do for the reasons above, in OPEN QUESTIONS. But it's an option.

[kevmoo]

but I think an order of magnitude less than unawaited.

Less than unawaited maybe, but pretty common. I think I hit these about as often as I hit unawaited.

[gspencergoog]

This doesn't feel like an API that will change much (it mirrors the import mechanism, which is pretty mature syntax), so I don't have a lot of concern about the API needing to wait for major releases to make changes. It's also given meaning by the tools, since it's mostly just metadata, so the tools can evolve around this basic definition without needing the definition itself to change. And, since it's basically the same information as in an import statement, it's not really tool specific: someone could write another documentation system, and they'd still need, and be able to use, this doc import information.

@docImport declared in package:meta and I think it would also have to be declared in dart:ui, so that flutter/engine could use the annotation. The analyzer would have to recognize annotations from both libraries.

I'd prefer it to be in dart:core, for the reasons Sam mentioned initially, but if that's not acceptable, It seems like this is the least bad alternative.

[lrhn]

As a counter-point to my other opinion, the one reason to put annotations into platform libraries is that the libraries themselves use those annotations (Deprecated, Since, pragma).

Do we expect that platform libraries will want to use the feature?

Platform libraries may want to refer to, say package:async or package:collection, but definitely do not want to depend on those for compilation. A doc-dependency may be just the thing we need for that. If we do use an annotation for it, then we will need that annotation in the platform libraries too. (Obviously, if we use // @doc import ..., then you don't need anything to write it, it's done entirely inside the DartDoc tool. But then, so is the annotation interpretation, it's just the annotation class itself which needs to exist.)

I still think making documentation depend on code-level declarations is breaking out of the documentation. Needing a dependency to write comments (and not just on the thing you want to link to) feels like overkill,

[srawlins]

Do we expect that platform libraries will want to use the feature?

dart:ui is the primary motivation for putting this in the platform libraries. I'll leave it to others to declare whether that counts as being in the platform libraries. As I understand it, the code that makes up dart:ui cannot depend on packages.

[lrhn]

My gut feeling is that the most appropriate approach is to put the links inside comments (maybe even require it to be a doc-comment, so /// @doc import ...). That way, they are only visible to DartDoc itself.

If it's an annotation, then it's visible to everybody, whether they need it or not. That's true for any other annotation too, but DartDoc already has its own carved out area for non-annotation metadata (doc-comments), so it feels unnecessary to add a second.

I'm not particularly worried about parsing. If we can piggy-back on the Dart parser, it's better, but even without it, it's a fairly restricted grammar.

If we want full IDE integration, then the analyzer should be able to understand the imports too (and therefore parse them), so that it can correctly link external doc-references in an IDE. I'd prefer to have that no matter what syntax we choose.

[srawlins]

To me, dartdoc is only half, maybe less than half of the story here (this proposal hardly references dartdoc). The main client, in my mind, is analyzer, such that these references inside doc comments are resolved correctly inside the IDE (meaning lots of things: navigation, hover, participation in renames, etc), in static analysis, in code search (GitHub, Kythe, etc)

That being said, the singular use is, as you point out, to handle comments, which is not something typical annotation-customers handle. I don't know if the language spec or any other document says what annotations are for, or if their use is encouraged for this purpose or that purpose, but today most annotation use is for code generation (via analyzer APIs) or the analyzer (as its own tool).

[lrhn]

The intended use of annotations is to attach metadata to something. The spec doesn't say any more, because it cannot know. Attaching metadata only makes sense if someone can read it. With dart:mirrors being gone on most real platforms (web and Flutter), annotations are primarily read by source-processing tools.

The design of annotations, however, was guided by being used by mirrors. That's why it's important that the value can be created using actual code imported into the current program, because otherwise the value couldn't exist in the program when dart:mirrors accessed it. If I were to design metadata today, I'd probably not do it that way. Using Dart objects is a reasonable choice, because it gives structure and type checking that would otherwise become a second mini-language, but I'd probably introduce "meta imports" to back the annotations, which the production compiler would not need to worry about. Members only imported using meta-imports could not be accessed outside of metadata. Maybe even allow more separation, so annotation processing tools only need to care about their own imports, not those of other annotations. Kind-of like what I think we're doing with macros. And what @docImport would do, but without being reusable for other annotations,.

DartDoc is a little special in this regard in that it's a kind of metadata attached to declarations, intended for a tool to process, that predates annotations. Even if we did have annotations back then, documentation is special, because it's more like text than like structured data. It wouldn't be as convenient if you had to do @dartdoc(""" .... docs ... """) instead. We'd probably still use comments for documentation, like every other language.

So, annotations are not amazing, because they impose source dependencies that the production code doesn't need. DartDoc has a way around that by embedding its own metadata inside the comments, like macros, which is why I'd lean into that instead of introducing another external code dependency, just to be able to interpret the doc contents. In that way doc-imports feel similar to doc macros, which are required to interpret later DartDoc content, but not used for anything else. Doc-macros could also have been annotations, but it didn't seem reasonable (probably also because they contain actual documentation text).

Putting the dependency into dart:core sidesteps adding a package or library dependency, but still moves something related to DartDoc content out of the doc comments where I'd look for it.

[gspencergoog]

I do wish that Dart source files were able to integrate foreign data containers more easily into the language.

For instance, it would be nice to have the ability to segregate portions of the file (without needing to prefix every line) to enable things like building the equivalent of a Jupyter notebook or CoLab, where the code is effectively embedded in the documentation instead of the other way around.

Or being able to have multiple compilation units in one file, so that you can have unit tests, API documentation example code, and application/framework code co-mingled in the same file in different namespaces, and have the analyzer (and thus IDEs) understand all of them and check them in their respective namespaces/containers.

Although, really, that isn't necessarily the purview of the language to define: you could build those things on top of the Dart language as-is with tools (as we've done with dartdoc), and that probably allows better subdivision of responsibility and allows more powerful aggregation of fine grained tools. But it is helpful to be able to attach extra-language metadata in the file to serve as data sources, which is kept in sync with the code, for the tools operate on.

The /// @doc import syntax isn't horrible (although I'd prefer /// @import for brevity), and I can see the argument for "keeping it in dartdoc", but what it means is that the analyzer is going to need to parse and understand not only Dart, but also more and more of the documentation comments as if they were required to be in dartdoc format, making the analyzer more complex than it already is, and importing more of this documentation mini-language into the analyzer. It seems like it would be nice if the documentation formatting/understanding part of the analyzer could be confined to an analyzer plugin so that theoretically multiple different plugins could be created for different documentation sub-languages (don't you want to write API docs in LaTeX or postscript? :-) ).

This extra dependency information seems a little different than the comments themselves, however, and more in the analyzer's wheelhouse to resolve. If you want to aggregate small tools or plugins instead of making the analyzer a monolith, it seems reasonable to provide the different symbol lookup namespaces to a documentation-formatting analyzer plugin as input rather than letting the plugin figure it out for itself from its own metadata.

Maybe a solution that would keep documentation parsing separate from the main analyzer while not involving the @docImport syntax would be to have the analyzer provide an API to plugins for resolving symbols using an alternative set of imports (if that doesn't already exist) after parsing the extra imports out of its own metadata format.

[srawlins]

The /// @doc import syntax isn't horrible (although I'd prefer /// @import for brevity), and I can see the argument for "keeping it in dartdoc", but what it means is that the analyzer is going to need to parse and understand not only Dart, but also more and more of the documentation comments as if they were required to be in dartdoc format ...

Yeah this actually would be all implemented in the parser, just as doc comment references are parsed in the parser. The CFE (I imagine) just drops that stuff on the floor, but it does tokenize it, and we'd apply the same functionality to a doc import directive inside a comment.

Maybe a solution that would keep documentation parsing separate from the main analyzer while not involving the @docImport syntax would be to have the analyzer provide an API to plugins for resolving symbols using an alternative set of imports

I think would be very complicated. We don't provide any APIs for resolving elements.

[srawlins]

Thinking and playing in the parser, I have some notes on the doc comment (/// @doc import) support:

• Currently the Fasta tokenizer tokenizes single-line (/// or //) comments into individual lines: A 3-line doc comment like /// one\n/// two\n/// three\n is tokenized into 3 tokens: /// one\n, /// two\n, /// three\n. There are hooks (if my understanding is correct) for a front-end like analyzer to tokenize more tokens, which is how comment references are broken up into more tokens (analyzer's parseCommentReferences calls scanString to tokenize.

  I think we'll need to tokenize doc imports in the same way. There is a benefit here in that the front_end will (I imagine) not bother tokenizing. Analyzer, on the other hand, can tokenize and parse a doc import.

• One hiccup here is that a doc import should be able to extend over multiple lines, and the /// bits must be tokenized too:

  /// @doc import 'package:foo/foo.dart'
  ///     as foo;
  /// @doc import 'package:bar/bar.dart'
  ///     show
  ///       One,
  ///       Two,
  ///       Three;

  I imagine we can tokenize the first one, as an example, into @doc import 'package:bar/bar.dart' /// show /// One, /// Two, /// Three; (modulo whether the little bits get their own tokens, like @ or ,, I'm not sure). Notice the /// tokens; not sure how these will be encoded...

  Regardless it means we probably cannot simply tokenize with scanString.

[bwilkerson]

... modulo whether the little bits get their own tokens, like @ or ,, I'm not sure ...

If we're using the Scanner, which I think we should, then yes, they will be separate tokens, which is appropriate. We'd basically parse until we get to a ;. Of course, we'll need to decide what to do if we don't find a ; (or if there are other parsing errors in the imports).

It makes me wonder just how critical it is for doc imports to be able to extend across multiple lines (or have combinators, prefixes, or configurations, for that matter; I assume we won't support deferred).

Regardless it means we probably cannot simply tokenize with scanString.

We probably can, we'd just need to remove the leading ///s or *s first in order to not scan them as comments.

[srawlins]

@yjbanov: I was thinking about this with @gspencergoog, and our latest idea is writing an annotation class in dart:ui and package:meta. They could both be named @docImport. The one in dart:ui does not need to be exported from any public library, so it can be "internal-only" (even @internal I suppose).

Analyzer can recognize both of the locations of @docImport and treat them identically, to improve doc comment references, and also first class support in the annotation itself. Meaning this code:

@docImport('package:flutter/foundation.dart', show: ['Widget', 'Elemet'])

can have IDE support:

• hover and jump-to support on the URI
• hover and jump-to support on 'Widget'
• error reporting on 'Elemet' (typo)

etc.

[DanTup]

our latest idea is writing an annotation class in dart:ui and package:meta. They could both be named @docImport. The one in dart:ui does not need to be exported from any public library, so it can be "internal-only" (even @internal I suppose).

Perhaps a can of worms, but sometimes when I use annotations the server recognises, they remind me of some things in .NET where it didn't matter where an Attribute was defined, only its name. This meant you could often avoid dependencies on extra libraries in your production code by just defining your own internal attribute with the same name.

I've often wondered if the same could work in Dart. For example if I made my own @deprecated and put it on a class, could I avoid needing package:meta? Things may get a little trickier when they have fields, but as long as the analyzer could read them and the rules are clear, maybe it could work?

Each time I see Flutter special cased somewhere, I wonder if someone wanted to make a "Flutter-like" framework, how many places they'd find they can't get the same functionality because of this. Are the reasons that Flutter can't use package:meta particular to Flutter, or might some other project have the same restrictions?

Btw, I'm curious if just using normal imports with some modifier/annotation (@visibleForDocs?) was considered. I suspect it's ruled out for the same reason as not putting the annotation in dart:core, but if this feature is going to have full IDE support for finding references, hovers, renames, etc., it will feel like a first-class Dart feature and having different two ways to "reference" files might feel a little odd.

[srawlins]

For example if I made my own @deprecated and put it on a class, could I avoid needing package:meta?

Yeah that's nearly what I'm proposing, haha. I think in analyzer we only validate, for example, a @protected annotation if it is the protected const from package:meta, because package:meta is generally available everywhere (hold that thought), and so if some code generator or other defines their own @protected for their own reasons, we should keep our hands off it.

Are the reasons that Flutter can't use package:meta particular to Flutter, or might some other project have the same restrictions?

I'm not sure the technicalities behind it, but basically any dart: library, include "Flutter engine" (dart:ui), cannot reference a package library, like package:meta/meta.dart. @yjbanov actually mentioned that they do import package:meta for their dev cycle, but when the engine is published as dart:ui, all references to package:meta and the various annotations are stripped from the code. That doesn't help us in this case where we want to ship the engine with @docImport annotations intact.

I'm not even sure why the flutter engine is shipped as dart:ui, and not a package library, but I imagine there are good reasons. And if some 3rd party wanted to build their own platform on top of Dart, similar to Flutter, and they shipped their own dart:* library, then yes, they'd have the same problem ☹️ .

Btw, I'm curious if just using normal imports with some modifier/annotation (@visibleForDocs?) was considered. ...

I think the general reason is that IDE support is not good enough to make something a language feature. If it's not even good enough a reason to add to dart:core, I don't see it making it into the language.

[DanTup]

and so if some code generator or other defines their own @protected for their own reasons, we should keep our hands off it

Yeah, that's probably fair. I can't remember examples where I'd used this in .NET, but I think they were probably much more specific names than things like @protected so accidentally overlapping was probably much less likely.

Thanks for the extra info!

[devoncarew]

I'm coming into this conversation a little late, but the idea of using the @pragma annotation appeals to me.

@pragma('dartdoc:import', 'package:flutter/material.dart')

import 'dart:convert';
import 'dart:io';

import 'package:path/path.dart' as p;

...

• it already exists in our core libraries
• it's meant to be used for tooling
• annotations are more parsable than custom conventions in code comments
• the primary consumer here may not really be dartdoc, but the analyzer / linter / analysis server. Ideally linting and navigation (and code completion?) would be as aware of these auxillary imports, as much as dartdoc itself would be

[lrhn]

If possible, would you want to use @pragma('dartdoc:import', '....') as the base, and have a public

class DocImport extends pragma {
  const DocImport(String import) : super('dartdoc:import', import);
}

(If so, I'll have to remove the final from class pragma. So far people haven't been happy about extending pragma.)

[DanTup]

If so, I'll have to remove the final from class pragma.

Wouldn't the pattern above be useful even if not used for DartDoc? I didn't know about pragma before, but it seems quite useful and I could imagine that wrapping it like that could be useful for other tools that want to use it.

[lrhn]

I'm not sure it's actually useful. I've thought about it some more, and I think it's better to keep pragma as final.

Using a @pragma("dartdoc:import", "...") inside the platform libraries and a @DocImport("...") outside of the platform libraries can be useful, but it's not a given that base class DocImport extends pragma { } is necessary or desirable.

It gives you an implementation approach, which is to detect whether the annotation extends pragma, and then reading its fields for data, making sure to accept both instances of pragma and the subclass DocImport, and maybe even further subclasses. And you'd still check the name property, even on DocImport, because maybe some subclass overwrote the field.

In the same code, you could probably just as easily check for whether the annotation was an instance of one of two final and unrelated classes (pragma or DocImport), only check the name property of the pragma, and you can trust that the fields are not overridden.

So, all in all, I don't think subclassing pragma is the best design choice. It leaves too many things open that you'll then have to check in the annotation recognition code. Going for final everywhere makes that easier.

[DanTup]

FWIW, I was expecting checks to be on the name and thought the subclass was nice just for convenience of the user adding the annotation. If I create a tool FooTool and wanted people to use a pragma annotation, wouldn't it be more convenient for them to do:

@fooTool('xyz')

than:

@pragma('FooTool', 'xyz')

Both should work (FooTool should only check name), but the top one gets code completion with a nice dartdoc comment on the completion list explaining how to use it and what it does. The second gets no completion on the important string, and no documentation explaining the use.

(Note: I don't have any current use cases for pragma so is just me thinking out loud - this idea might be flawed in other ways :-) )

[bwilkerson]

I had the impression that pragma was intended to be used to provide additional information to the compilers. I guess I assumed that the analyzer wouldn't be considered to be part of that use case. Perhaps that assumption was wrong.

Whichever way we implement doc imports we might want to consider adding tooling support for pragma, assuming that it's used often enough. If there's some metadata maintained somewhere about the valid values for the first argument, then we could provide code completion even within the string. If we have a complete list, we could provide a warning when the string isn't one of the valid options. If there were docs associated with the metadata, then we could display it in hover text.

[devoncarew]

For a flavor of how pragma's being used now, here are the unique uses in the sdk repo:

@pragma('dart2js:${annotation.name}')
@pragma('dart2js:${other.name}')
@pragma('dart2js:...')
@pragma('dart2js:<suffix>')
@pragma('dart2js:as:check')
@pragma('dart2js:as:trust')
@pragma('dart2js:assumeDynamic')
@pragma('dart2js:disable-inlining')
@pragma('dart2js:disableFinal')
@pragma('dart2js:downcast:check')
@pragma('dart2js:downcast:trust')
@pragma('dart2js:index-bounds:check')
@pragma('dart2js:index-bounds:trust')
@pragma('dart2js:late:check')
@pragma('dart2js:late:trust')
@pragma('dart2js:load-priority:high')
@pragma('dart2js:load-priority:high)
@pragma('dart2js:load-priority:normal')
@pragma('dart2js:load-priority:xxx')
@pragma('dart2js:never-inline')
@pragma('dart2js:noElision')
@pragma('dart2js:noInline')
@pragma('dart2js:noSideEffects')
@pragma('dart2js:noThrows')
@pragma('dart2js:parameter:check')
@pragma('dart2js:parameter:trust')
@pragma('dart2js:prefer-inline')
@pragma('dart2js:resource-identifer')
@pragma('dart2js:resource-identifier')
@pragma('dart2js:tryInline')
@pragma('dart2js:types:check')
@pragma('dart2js:types:trust')
@pragma('dart2js:unknown')
@pragma('flutter:keep-to-string-in-subtypes')
@pragma('m:never-inline')
@pragma('OtherTool:other-pragma')
@pragma('Tool:pragma-name',
@pragma('vm-test:can-be-smi')
@pragma('vm:entry-point')
@pragma('vm:entry-point',
@pragma('vm:entrypoint')
@pragma('vm:exact-result-type',
@pragma('vm:external-name',
@pragma('vm:ffi:native-assets',
@pragma('vm:invisible')
@pragma('vm:isolate-unsendable')
@pragma('vm:never-inline')
@pragma('vm:non-nullable-result-type");
@pragma('vm:notify-debugger-on-exception')
@pragma('vm:prefer-inline')
@pragma('vm:testing.unsafe.trace-entrypoints-fn',
@pragma('vm:testing:match-inner-flow-graph',
@pragma('vm:testing:print-flow-graph')
@pragma('vm:testing:print-flow-graph',
@pragma('vm:testing:print-flow-graph'[, ...
@pragma('vm:unsafe:no-interrupts')
@pragma('wasm:entry-point')
@pragma('wasm:prefer-inline')
@pragma('weak-tearoff-reference')

[DanTup]

The docs at https://api.dart.dev/stable/2.19.6/dart-core/pragma-class.html suggest to me that this is open for any tools to use. If it's intended to only be Dart SDK tools, it should probably be clearer. If not, I guess the strings could be anything (and having a subclass may add value as noted above?)

[bwilkerson]

... here are the unique uses in the sdk repo:

A couple of those appear to be for tests ('OtherTool:other-pragma' and 'Tool:pragma-name'), but the rest match my previous understanding.

If not, I guess the strings could be anything ...

Which is, of course, the problem with strings. Any two tools could start using the same string and there'd be no way to know until it caused problems for one or more users. There's value is having namespaces, and from that perspective alone I'd rather define a unique annotation somewhere that we can easily uniquely identify.

[srawlins]

If we really don't want to add DocImport to dart: and want to keep pragma final, my plan would be to implement this instead:

Add DocImport to two locations: the meta package, and the flutter engine. The one in the flutter engine would be used in the flutter engine, but not exported from dart:ui. It would hopefully just look like it was the same class as the one in the meta package. The definitions of the two classes would be subject to drift. Tools like the analyzer and dartdoc would recognize each class in annotations; analyzer could provide navigation, autocomplete, hover, etc. (but I don't anticipate this annotation being used in many locations, so not high priority).

This would support the primary use case, which is flutter engine. No code in the Dart SDK could use DocImport (obviously).

[lrhn]

We can add a DocImport to the SDK platform libraries too, without exposing it. Then dart:ui could use that, I just don't want to export it from the platform libraries. (The @Since annotation is not exposed either. It predates pragma, otherwise it might not have existed.)

The pragma class was intended for individual platform tools, not just compilers, that processes the platform libraries to be able to add metadata to the platform libraries, without needing to first introduce new classes that everybody else would need to care about. A VM specific annotation would be fine in a VM-only patch file, but it's just noise if it has to be declared where every other tool has to parse it too. And while the platform libraries cannot depend on third-party libraries, we also want to avoid having too many dependencies in other SDK code as well.

The pragma design, that the first string should be "tool-name:meaning" and the second argument can be any supplemental data, if any, makes for an open namespace, but well-behaved annotations can be easily ignored by other tools than the one it's intended for. If two tools insist on having the same name ... well, they can fight it out. And non-well-behaved annotations should just be ignore, and will never exist inside the SDK.

A @pragma('dartdoc:something', 'moredata') annotation is fine. It's rare that the platform libraries have special dart-doc needs (I think, but not unheard of), so there probably hasn't been as much demand for such an annotation.

A @pragma('dartfmt:ignore-file') annotation would make perfect sense for some of our syntax tests.

---

Looking entirely at dependencies, I'd prefer that a dart-doc import was inside the dart-doc itself, as // @import ...;. That has zero dependencies on anything else, and I don't think dart-doc should have dependencies. Having to import package:meta in order to not have to import another library still seems like missing the point.

It has to be parsed, but I'm sure that can be solved - trailing semicolons have a purpose. (We just need to make sure that a syntax error in doc-comments doesn't block anything else from working.)

If the analyzer also needs to know about the imports, then it too has to parse the doc-import. But it only needs to know about the imports because it parses doc-comments to begin with, so it's fair that it parses all of it.

If that's not acceptable, we can always add a declaration in package:meta. If we want platform libraries to use it too, we can either add a separate declaration in dart:_internal (usable by dart:ui as well), or we can just tell platform libraries (including dart:ui) to use a pragma. Either works, and in both cases, it's a seprate class from the one in package:meta.

[srawlins]

Looking entirely at dependencies, I'd prefer that a dart-doc import was inside the dart-doc itself, as // @import ...;.

I prefer this as well. I just would be a poor engineer choice to implement it (I tried and commented here). I was trying to be pragmatic about it in suggesting a much more simple (but I agree, less good) annotation.

[lrhn]

Seems like it should be possible to filter a token-stream to include only the content of the doc-comment. After all, detecting comment-prefixes is how you know where the comment ends.

So, given a token stream of "///, more, more, \n, /// more, more, \n, class ...", the filtered token stream would be just "more, more, more, more". I don't know how that'd work technically, if the same token can be part of two streams, but logically, passing tokens of the content of the comment into into the dart-doc parser seems like just the right thing to do. After all, the /// and * are not part of the documentation, they're just delimiters. (Or maybe just don't tokenize comment contents in the same pass as the rest, find the comment extent first, then re-scan it afterwards with a doc-scanner, not a Dart scanner.)

[bwilkerson]

As I mentioned before (https://github.com/dart-lang/sdk/issues/50702#issuecomment-1370023433), scanning across multiple single-line comments isn't a problem. We trim the source of the comments to rescan only the portion between square brackets when dealing with doc references, and this wouldn't be any harder.

[srawlins]

Here's an update on what's been implemented, as it's not quite what's specified at top, CC @kallentu:

• We're going with a comment-based implementation instead of an annotation-based implementation. It looks like:

  /// @docImport 'package:foo/foo.dart';
  /// @docimport 'bar.dart' as bar show Bar;

• These doc imports are attached to the Comment node, in the docImports field.

• Doc imports are currently not found on a LibraryElement, but I think they'll need to be.

• I have a pending CL that incorporates doc imports into the element model, FileState, and more.

Otherwise, I think the resolution steps specified up top, and the new errors that should be reported, are unchanged.

[bwilkerson]

Doc imports are currently not found on a LibraryElement, but I think they'll need to be.

To clarify, are you talking about having the doc namespace available from the LibraryElement, or do you mean the actual imports (or both)?

I haven't compared closely, but are the rules for doc imports the same as for normal imports (modulo the namespace being computed), or are there some interesting differences?

when elements from a doc import have name conflicts with each other, or elements from imports with the same prefix

I believe that the rules for normal imports is that it's only an error if the conflicting name is actually referenced. I assume the same is true here.

[srawlins]

I haven't compared closely, but are the rules for doc imports the same as for normal imports (modulo the namespace being computed), or are there some interesting differences?

Yes I think they're exactly the same.

I believe that the rules for normal imports is that it's only an error if the conflicting name is actually referenced. I assume the same is true here.

Ah yes, fair. True here as well.

[srawlins]

@lrhn To complete this work, I think it would be consistent, simple and straightforward for the analyzer will start reporting "compile time errors" on doc-imports, the same that are reported for regular imports. WDYT of that?

For example, /// @docImport 'does-not-exist.dart'; will result in a compile-time error. Is this over-stepping our purview, or good for consistency? We could probably use "warnings" (previously, hints) that mirror such compile-time errors, if we should not actually be reporting compile-time error codes. Something like a dozen different errors can be raised on imports I think.

[lrhn]

I'd be fine with reporting an invalid import as an error. I'd also make it an error to have a [Foo] where Foo cannot be resolved. But I'm usually fine with breaking bad code. Others might be more cautious. Might want an introductory period, maybe a single release, where it's only a warning with a message saying that in the next release it will become an error.

(I'm a little iffy about calling it a "compile-time error" for a tool that doesn't actually compile, but that's probably just idle pedantry. It's really a "doc-time error", where the analyzer is reporting that that the DartDoc tool will have an error here, just like a compile-time error is the analyzer reporting that a compiler will have an error. Or it's just "an error" because someone wanted to make something not be accepted, aka. lints).

[srawlins]

Superceded by https://github.com/dart-lang/sdk/issues/56186, which includes the new syntax, a directive in doc comments, as implemented in the analyzer.