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

MosheBerman abandoned this revision. MosheBerman added a comment. In the interest of not leaving detritus on the internet, I'm going to abandon this diff. I believe there's an alternate approach worth pursuing, with clang tidy. Thanks for your reviews and constructive feedback! Repository:

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

MosheBerman updated this revision to Diff 430691. MosheBerman added a comment. Missed a whitespace. (Looks like there may be another merge conflict, in NullabilityChecker.cpp:117, but not sure if that's the issue. Let's try this just in case it's not.) Repository: rG LLVM Github Monorepo

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

MosheBerman updated this revision to Diff 430686. MosheBerman marked an inline comment as not done. MosheBerman added a comment. My IDE auto-formatted away some whitespace, causing a merge conflict for the test CI. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

MosheBerman updated this revision to Diff 430408. MosheBerman marked an inline comment as done and 2 inline comments as not done. MosheBerman added a comment. Addressed feedback. - Removed comment about NS-classes - Changed some of the LIT test invocations - Added StringRef where appropriate.

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

MosheBerman updated this revision to Diff 429068. MosheBerman added a comment. Update tests to use %clang_analyze_cc1. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123352/new/ https://reviews.llvm.org/D123352 Files:

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

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:

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

MosheBerman updated this revision to Diff 429065. MosheBerman added a comment. Incorporated feedback: - Removed confusing comment about NS-prefixed classes - Use StringRef instead of str - Fixed case where `isa` was more appropriate Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

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

MosheBerman marked 3 inline comments as done. MosheBerman added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:592-602 + // If we're inside of an NS_ASSUME, then the sourceRange will end before the + // asterisk. + const auto

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

MosheBerman added a comment. In D123352#3475889 , @NoQ wrote: > I'm worried that even if the warning is correct, the suggested fix is not > necessarily the right solution. > > The very nature of path-sensitive warnings requires multiple events to happen

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

MosheBerman added a comment. In D123352#3474033 , @whisperity wrote: > Regarding FixIts... FixIts are implemented in the "Diagnostic" library, which > is non-specific to neither Clang-Tidy nor Sema whatsoever, they use the same > infrastructure under

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

MosheBerman planned changes to this revision. MosheBerman added a comment. Thanks for all the thoughtful and fantastic feedback! Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:592-602 + // If we're inside of an NS_ASSUME, then the sourceRange will end

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

MosheBerman updated this revision to Diff 424048. MosheBerman added a comment. Fix tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123352/new/ https://reviews.llvm.org/D123352 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

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

MosheBerman added a comment. In D123352#3458530 , @steakhal wrote: > In D123352#3439649 , @MosheBerman > wrote: > >> In D123352#3439390 , @steakhal >> wrote: >> >>>

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

MosheBerman updated this revision to Diff 424010. MosheBerman edited the summary of this revision. MosheBerman added a comment. - Improved handling of `NS_ASSUME` and other edge cases/ - Added test cases, and changed the test to actually validate the output (not just the presence of fixits.) -

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

MosheBerman added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:162-164 + /// 2. If we are annotating an Objective-C method, and not a function, we + ///want to use the `nullable` form instead of `_Nullable`. + ///When \p

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

MosheBerman updated this revision to Diff 422922. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123352/new/ https://reviews.llvm.org/D123352 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

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

MosheBerman updated this revision to Diff 422902. MosheBerman added a comment. The tests now pass and actually verify the behavior. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123352/new/ https://reviews.llvm.org/D123352 Files:

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

MosheBerman added a comment. Hey, bumping for feedback, please. :D Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123352/new/ https://reviews.llvm.org/D123352 ___ cfe-commits mailing list

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

MosheBerman updated this revision to Diff 421587. MosheBerman added a comment. - Changed the ShowFixIts flag to be per-checker. - Added support for syntax sugar (`nullable` vs `_Nullable`) - Pass FixItHint through, so we can do that. - Changed tests to use `-fdiagnostics-parseable-fixits`.

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

MosheBerman added a comment. In D123352#3439390 , @steakhal wrote: > tldr; static-analyzer fixits are not completely implemented. Where can I learn more about this? Would it be possible and idiomatically/architecturally sounds to write a clang-tidy

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

MosheBerman added a comment. In D123352#3438475 , @steakhal wrote: > You have a single bool property, yet you allow to enable/disable this by each > sub-checker. It feels like the last checker in the registration process will > rule them all. > > That

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

MosheBerman created this revision. Herald added subscribers: manas, steakhal, ASDenysPetrov, martong, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: All. MosheBerman requested review of this revision. Herald added a