[PATCH] D123352: [analyzer] Add FixItHint to `nullability.NullReturnedFromNonnull` and `nullability.NullableReturnedFromNonnull`

[Moshe via Phabricator via cfe-commits]

MosheBerman added inline comments.


================
Comment at: clang/test/Analysis/nullability-fixits.mm:5-10
+// RUN: %clang_cc1 -analyze \
+
+// RUN: -Wno-nonnull \
+// RUN: -analyzer-checker=core,nullability.NullableReturnedFromNonnull,nullability.NullReturnedFromNonnull \
+// RUN: -fdiagnostics-parseable-fixits %s 2>%t
+// RUN: cat %t | FileCheck %s -check-prefix=CHECK-FIXIT-DISABLED-IMPLICIT
----------------
MosheBerman wrote:
> steakhal wrote:
> > Why don't you use the `%clang_analyze_cc1 ...` form instead or even better the `%check_analyzer_fixit` tool-subst pattern?
> > See the `clang/test/Analysis/dead-stores.c` as an example.
> That’s a good call out. I looked at that exact test and was having trouble testing. Instead, I implemented the RUN commands from the most basic invocation I could find. 
> 
> I’m happy to update the tests that verify that fixits are emitted. For the one that checks that fixits are applied, is there a better way? The `-verify` flag doesn’t do that, from what I saw in the docs. 
> Why don't you use the `%clang_analyze_cc1 ...` form instead or even better the `%check_analyzer_fixit` tool-subst pattern?
> See the `clang/test/Analysis/dead-stores.c` as an example.

I tried both, and `%clang_analyze_cc1` will work. The `check_analyzer_fixit` pattern fails because the test imports a fake header, which exists at input, but not in the output location.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123352