Recognize multi-line TODO comments

It's great that we recognize // TODO:! It would be even awesomer if we were able to concatenate multi-line TODO comments. Right now they're cut off at the first line break, for example:

@bwilkerson

To do(!) this, I think we'd need to make some assumptions about whether the following lines are part of the TODO (as opposed to existing comments the TODO was added above). I had a quick scan through the SDK and mostly what I found seems to be multiline TODOs, but I did see some instances of code that was commented out with a TODO above it:

// TODO(dantup): Fix this
// if (blah) {
//   brokenFunction();
// }

I don't know how common that is generally (and squiggling(?) the full code sample might look bad), of if there are any conventions that could be used to make this distinction clear.

@bwilkerson has this come up/been discussed before?

Yeah -- there's some good prior art here, I think. Here's IntelliJ:
https://stackoverflow.com/a/53472479

Ah, nice! All the ideas I had (like putting a dash on the end of the TODO line) sucked, but that seems pretty reasonable. If ther are no objections/concerns, I'll take a look (next year!). Thanks!

I've been wanting for a while to make time to implement the IntelliJ semantics, but haven't yet. I'd be happy to have you look at it, though it should be done in a way that will work for both protocols. (IntelliJ users loose this feature when working with Dart code because server overwrites the highlighting.)

I'd be happy to have you look at it, though it should be done in a way that will work for both protocols

Yep, I was thinking it could be implemented in here where it should apply to both:

https://github.com/dart-lang/sdk/blob/d63d5d93a59b370bf5f5c277109110cb67a96b75/pkg/analyzer/lib/src/error/todo_finder.dart#L34

Currently _scrapeTodoComment processes a single token (and its caller, _gatherTodoComments loops through them), but perhaps _scrapeTodoComment could read ahead to append more text, and return the next token to process instead?

I agree that todo_finder.dart is the right place to implement this. I don't have any strong preference for how it gets implemented. I'm sure you'll figure out what works best when you start working on it.

@bwilkerson

@bwilkerson I've got a rough version of this working:

https://dart-review.googlesource.com/c/sdk/+/178680/

However there's a case I'm unsure how to handle:

// TODO: aaa
//  a

//  b

Here, there are three single line tokens and the 2nd/3rd look like candidates to unwrap (they're both indented extra). Really we don't want to roll over a blank line, however I can't see an easy way from the tokens to know that there's a blank line between (I thought I could compare the end/offsets, however the newlines aren't included so it's not possible to tell the difference between \n\n and \r\n).

It seems like a bit of an edge case, but there's some code in the SDK that triggered it when testing so it might be common in the wild too:

https://github.com/dart-lang/sdk/blob/d63d5d93a59b370bf5f5c277109110cb67a96b75/pkg/analyzer/test/src/lint/io_test.dart#L15-L17

Is there an easy way I could detect this?

If you have access to an instance of LineInfo then it can tell you which line each comment is on. That's the easiest way.

I thought I didn't, but turns out I do from the CompilationUnit. Thanks!

[analyzer] Add support for multiline TODO comments

This landed in dart-lang/sdk@6c33457 and should be available in Flutter master now. Do let me know if you spot any places where it doesn't do what you'd expect!

@bwilkerson

@bwilkerson I've got a rough version of this working:

https://dart-review.googlesource.com/c/sdk/+/178680/

However there's a case I'm unsure how to handle:
// TODO: aaa
//  a

//  b
Here, there are three single line tokens and the 2nd/3rd look like candidates to unwrap (they're both indented extra). Really we don't want to roll over a blank line, however I can't see an easy way from the tokens to know that there's a blank line between (I thought I could compare the end/offsets, however the newlines aren't included so it's not possible to tell the difference between \n\n and \r\n).

It seems like a bit of an edge case, but there's some code in the SDK that triggered it when testing so it might be common in the wild too:

https://github.com/dart-lang/sdk/blob/d63d5d93a59b370bf5f5c277109110cb67a96b75/pkg/analyzer/test/src/lint/io_test.dart#L15-L17

Is there an easy way I could detect this?

It still not working, for the first and second examples that you provided, using simple commenting //.

As you can see, I'm using one space after each line, and still not working.
On the other hand, I can acknowledge that it works for the third and fourth examples using /** */.

Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel stable, 3.3.1, on macOS 12.5.1 21G83 darwin-x64, locale en-IL)
[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0)
[✓] Xcode - develop for iOS and macOS (Xcode 13.4.1)
[✓] Chrome - develop for the web
[✓] Android Studio (version 2021.2)
[✓] VS Code (version 1.70.2)
[✓] Connected device (3 available)
[✓] HTTP Host Availability

• No issues found!

@CartmanGD

@CartmanGD are you referring to the blue colouring? I think you may have another extension installed that's doing that. The change I made above is for range reported on the diagnostics (which show on the Problems window, and usually have a squiggle under the text).

I can't see the squiggle in your screenshot, so can you check whether you have this setting enabled in VS Code:

@DanTup

Hey @DanTup thanks for replying. I'm using Android Studio, and the colors seem to reflect the todo which is presented on the Todos list section.

In other words, the first and second examples are not working in Flutter 3.3.1 (using simple commenting with space //). On the other hand, your third and fourth examples work fine using /* */.

Code screenshot:

Screenshot of the TODO list section on Android Studio:

As you can see, simple commenting // does not present the full description.

@CartmanGD

@CartmanGD ah, gotcha. I thought it was VS Code (there are some extensions that colour TODOs and other comments based on prefix, but they find their own ranges).

Do you see a difference if you capitalize TODO? For me, I only seem to get diagnostics when it's uppercase which makes me wonder if what you're seeing is coming from the IDE itself rather than the analysis server (see https://www.jetbrains.com/help/idea/using-todo.html).

There is no difference with capitalization, also I'm not using that "using-todo" plugin, so I assume it's from the IDE

@CartmanGD

@CartmanGD as far as I can tell, that is built-in functionality in IntelliJ/Android Studio and not a plugin. I was able to reproduce this. I created the following file:

void f() {
  // TODO: Capitalized
  //  across multiple lines

  // todo: first and second examples
  //  there is another space
  //  there is another space

  /*
    todo: third and fourth examples
     there is another space
     there is another space
   */
}

In the editor it looks like yours:

However if I log the server traffic, I only see this:

{
	"event": "analysis.errors",
	"params": {
		"file": "/Users/danny/Desktop/dart_sample/lib/todos.dart",
		"errors": [
			{
				"severity": "INFO",
				"type": "TODO",
				"location": {
					"file": "/Users/danny/Desktop/dart_sample/lib/todos.dart",
					"offset": 16,
					"length": 45,
					"startLine": 2,
					"startColumn": 6,
					"endLine": 3,
					"endColumn": 28
				},
				"message": "TODO: Capitalized across multiple lines",
				"code": "todo",
				"hasFix": false
			}
		]
	}
}

This only includes the capitalized TODO, and its range does span both lines. So as far as I can tell, the Dart server is providing the correct ranges to Android Studio (at least for capitalized TODOs), but Android Studio is doing its own rendering.

I do have the Android Studio setting described above ticked:

But it doesn't seem to work as expected. Unfortunately this seems like an IntelliJ/Android Studio problem and not something that can be fixed in the Dart server (at least as far as I know.. but maybe @alexander-doroshko would be able to confirm this).

I can confirm that this is controlled by the Dart plugin. The feature is tracked as https://youtrack.jetbrains.com/issue/WEB-34694/.

@alexander-doroshko

@alexander-doroshko Ah got it, thanks!

Navigation Menu

Repository navigation

Recognize multi-line TODO comments #3030

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Recognize multi-line TODO comments #3030

Description

Activity

DanTup commented on Dec 22, 2020

timsneath commented on Dec 22, 2020

DanTup commented on Dec 22, 2020

bwilkerson commented on Dec 22, 2020

DanTup commented on Dec 22, 2020

bwilkerson commented on Dec 22, 2020

DanTup commented on Jan 11, 2021

bwilkerson commented on Jan 11, 2021

DanTup commented on Jan 11, 2021

DanTup commented on Jan 19, 2021

CartmanGD commented on Sep 14, 2022

DanTup commented on Sep 14, 2022

CartmanGD commented on Sep 15, 2022

DanTup commented on Sep 15, 2022

CartmanGD commented on Sep 16, 2022

DanTup commented on Sep 20, 2022

alexander-doroshko commented on Sep 20, 2022

DanTup commented on Sep 20, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions