[PATCH] D65182: [analyzer] Add fix-it hint support.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ marked an inline comment as done.
NoQ added inline comments.


================
Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:491
+                       ArrayRef<SourceRange> Ranges = None,
+                       ArrayRef<FixItHint> Fixits = None);
 
----------------
gribozavr wrote:
> I'm not sure if this is the right model for fixits. Fixits are usually associated with a message that explains what the fixit does. Only in the unusual case where Clang or ClangTidy is very confident that the fixit is correct, it is attached to the warning. Most commonly, fixits are attached to notes.
> 
> Also, for IDE support, it would be really nice if we could provide short descriptions of edits themselves (e.g., "replace 'virtual' with 'override") that can be displayed to the user instead of the diff when possible -- right now we don't and tools using ClangTidy have to use a subpar UI because of that. For example, when we show UI for fixing a warning, displaying the complete diff is too much; a concise description would be a lot better.
> Fixits are usually associated with a message that explains what the fixit does. Only in the unusual case where Clang or ClangTidy is very confident that the fixit is correct, it is attached to the warning.

Wait, you guys already have fixits attached to notes? Then, yeah, i need to support this.

> Also, for IDE support, it would be really nice if we could provide short descriptions of edits themselves (e.g., "replace 'virtual' with 'override") that can be displayed to the user instead of the diff when possible -- right now we don't and tools using ClangTidy have to use a subpar UI because of that.

Mmm, interesting. I've seen this IDE of ours autogenerate such messages as "replace '`$code_in_removed_range`' with '`$inserted_code`'" (while also combining multiple parts of the fixit into a single replacement under the hood) and it looked quite bearable most of the time and i silently assumed that everybody does the same thing.

I agree that a high-level description of a fixit is nice to have. But given that you're attaching fixits to notes, isn't the text of the note text itself sufficient? E.g.:
- warning: variable may be used uninitialized here
- note: initialize 'x' here to suppress the warning
  - fixit: add ' = 0' after 'x'


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65182/new/

https://reviews.llvm.org/D65182