[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[Whisperity via Phabricator via cfe-commits]

whisperity created this revision.
whisperity added reviewers: aaron.ballman, alexfh, hokein, JonasToth.
whisperity added projects: clang, clang-tools-extra.
Herald added subscribers: gamesh411, Szelethus, rnkovacs, xazax.hun.
whisperity requested review of this revision.

While the original check's (D69560 <https://reviews.llvm.org/D69560>) purpose is to identify potentially dangerous functions based on the parameter types (as identifier names do not mean //anything// when it comes to the language rules), unfortunately, such a plain interface check rule can be incredibly noisy. While the previous //"filtering heuristic"// in D78652 <https://reviews.llvm.org/D78652> is able to find many similar usages, there is an entire class of parameters that should not be warned about very easily mixed by that check: parameters that have a name and their name follows a pattern, e.g. `text1, text2, text3, ...`. [1]

This patch implements a simple, but powerful rule, that allows us to detect such cases and ensure that no warnings are emitted for parameter sequences that follow a pattern, even if their types allow for them to be potentially mixed at a call site.

Given a threshold `k`, warnings about two parameters are filtered from the result set if the names of the parameters are either prefixes or suffixes of each other, with at most `k` letters difference on the non-common end. (Assuming that the names themselves are at least `k` long.)

The above `text1, text2` is an example of this. (Live finding from Xerces.)
`LHS` and `RHS` are also fitting the bill here. (Live finding from... virtually any project.)
So does `Qmat, Tmat, Rmat`. (Live finding from I think OpenCV.)

On average, **15%** reduction in findings were achieved with this feature turned on.

Similarly to the other filtering patch on the "relatedness modelling", this option is implemented as a check option, `NamePrefixSuffixSilenceDissimilarityTreshold`. If `0`, the heuristic is turned off. **Defaults to `1`**, i.e. a single letter difference ("pattern") on either end of the parameter name suffice for silence. This is the most reasonable default.

----

[1] It has been identified before by other research that such patterned names carry no viable information and such names are useless for a name-based analysis where argument vs. parameter names are contrasted with one another. [Rice2017]

//[Rice2017]//: Andrew Rice, et al.: Detecting argument selection defects. In: Proceedings of the ACM on Programming Languages, 1, pp. 104:1-104:23, 2017.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97297

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
  clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-prefixsuffixname.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D97297.325789.patch
Type: text/x-patch
Size: 13844 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210223/2811c11b/attachment-0001.bin>