Comments on directives are lost when running Organize Imports

[DanTup]

Given the code:

import 'dart:io';
// ignore: deprecated_member_use
import 'dart:async';

Stream<String> a;
File b;

If you sort it, the comment is removed:

import 'dart:async';
import 'dart:io';

Stream<String> a;
File b;

@bwilkerson I think this is because this code works by replacing the whole block of directives with a newly computed one, but it only captures end-of-line comments when it builds the new text:

sdk/pkg/analysis_server/lib/src/services/correction/organize_directives.dart

Lines 68 to 76 in 93313eb

	var end = directive.end;
	var line = lineInfo.getLocation(end).lineNumber;
	Token comment = directive.endToken.next.precedingComments;
	while (comment != null) {
	if (lineInfo.getLocation(comment.offset).lineNumber == line) {
	end = comment.end;
	}
	comment = comment.next;
	}

I think it could be easily extended to also capture preceding comments, however it might go bad if there's a "file header" comment before it:

/// Copyright ...
import 'dart:io';
import 'dart:async';

Some thoughts on fixing it:

• Only "attach" comments if it's not the first directive (this means a comment on top of the first directive will not move with the directive)
• Only attach // ignore comments (this might not solve the whole problem - eg. if the user added an explanatory comment to an import - // we're using this until the new library is ready).

What do you think?

[bwilkerson]

I suspect that one of the reasons this hasn't been implemented well is because of the difficulty of getting the right heuristics.

My opinion is that we should use the following heuristics:

• Every comment that follows a directive d that is on the same line as the last token of d (always a semicolon in valid code) should stay with d.
• Every comment that precedes a directive d and is not on the same line as last token of the previous directive, should stay with d unless
  • there is no library directive,
  • d is the first directive in the file, and
  • there is at least one blank line between the comment and d.

I can't think of any other special cases that might need to be addressed, but I'm sure we'll find or hear about some.

(Unfortunately for me, this code doesn't appear to be shared with the "sort members" feature. Perhaps if we fix this code someone can look at sharing the code between the two features.)

[lrhn]

(We really should allow library; without a name, so people have something to hang their library documentation off, rather than heuristically assume that a doc-comment on the first import is a library comment).

[bwilkerson]

It would be good to allow library; for cases where there is no need for other directives, but given the number of libraries that already make use of Dartdoc's heuristic, I doubt that we'll remove the heuristic. Certainly for now we need to handle that case correctly.

[DanTup]

🎉 Thanks @jonas-jonas!