preslavmihaylov / todocheck

A static code analyser for annotated TODO comments
MIT License
426 stars 42 forks source link

JSON output for closed-issue type comments doesn't include todo message #163

Open preslavmihaylov opened 3 years ago

preslavmihaylov commented 3 years ago

the output for // TODO 2: a closed issue is:

[
  {
    "type": "Issue is closed",
    "filename": "main.go",
    "line": 5,
    "message": ""
  }
]

in this case, the message a closed issue is missing in the json output.

bengsparks commented 3 years ago

Something quite tricky about extracting the TODO message from the comments is that depending on what type of comment the TODO is in, there may be tokens after the message itself.

/* TODO 2: a closed issue */

Simply stripping the TODO and Issue Ref from the comment is not sufficient, as the trailing */ will still be in the message. Furthermore, it is not possible to (trivially?) do this in a language-agnostic manner, as languages differ in comment syntax and can therefore have varying amounts of closing comment tokens.

The simplest suggestion I can offer is to count backwards from the end of the string and removing non-letter characters from the message until a letter is encountered. I am, of course, open to other suggestions :)

preslavmihaylov commented 3 years ago

Hmm, I see now that the original intent of this message was to print the detailed error message related to the err type.

For closed and non-existent TODOs, that message is not set as the error is evident from the error type.

I think it would be better to leverage this field for displaying the actual contents of the TODO instead.

Hence, there is no need to extract the message in the TODO itself. Instead, simply concatenate the TODO lines together with a \n and set it to the message.

bengsparks commented 3 years ago

Ok, I've done as you suggested. The output for single line comments looks good:

{
  "type": "Issue doesn't exist",
  "filename": "testing/scenarios/custom_todos/groovy/main.groovy",
  "line": 21,
  "message": "// TODO 3: The issue is non-existent",
  "metadata": {
    "issueID": "3"
  }
},

but the output for multiline looks a little odd.

{
  "type": "Issue doesn't exist",
  "filename": "testing/scenarios/custom_todos/groovy/main.groovy",
  "line": 56,
  "message": "/**\u0000* TODO 2: Invalid todo as issue is closed\u0000*/",
  "metadata": {
    "issueID": "2"
  }
},

As this is the first working iteration, I've opened a PR where we can discuss this further if necessary.