[PATCH] D82824: [clang-tidy] Added option to readability-else-after-return

Tue Jun 30 07:01:05 PDT 2020

aaron.ballman added inline comments.

================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-else-after-return.rst:74
+
+   When `true`, The check will attempt to refactor a variable defined inside
+   the condition of the ``if`` statement that is used in the ``else`` branch.
----------------
njames93 wrote:
> aaron.ballman wrote:
> > The -> the
> > 
> > I'm a bit unclear on what "attempt to refactor" means -- I sort of understand it to mean that if this option is true then the check will not produce a fix-it for variables defined inside the condition of the if statement that is used in the else branch, but will produce a diagnostic. However, the check behavior seems to also remove the diagnostic in this case (not just the fix-it), so I'm not certain I'm reading this right.
> Good spot, the check behaviour is also removing the diagnostic as well as the fix it.
> That behaviour should probably be changed to removing the Fix-It when this option is `false`, but then diagnostic behaviour should follow what `WarnOnUnfixable` dictates.
I think that makes sense. Then the option can be renamed to remove the "Refactor" bit and probably be something more like `WarnOnConditionVariables` or something?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82824