[clang-tidy] Add 'readability-suspicious-call-argument' check

[whisperity]

Summary

Find function calls where the call arguments might be provided in an incorrect order.
The check works by comparing the name of the arguments with the name of the parameters in the visible function declaration called.
A diagnostic is emitted if an argument name is similar to another parameter more than the one it is passed to currently, while also being dissimilar enough from the current one.

Several string metrics are implemented, and each has thresholds configurable by the user.

As this is a heuristics-based check, no FixIts are generated, on purpose.
False-positive (in the sense that the diagnostic isn't indicating an actual swap that was done) warnings from this check are still useful for developers as the findings indicate potential bad naming conventions used for variables and parameters.

---

Originally implemented by @varjujan as his Master's Thesis work.
The check was subsequently taken over by @barancsuk who added type conformity checks to silence false positive matches.

[alexfh]

Thank you for the new check!

Before starting with the review, I'd like to clarify one important thing. It's not immediately obvious that the pattern the check detects is actually a good indicator of a programming mistake. Did you try to run the check on a large enough codebase (at least, on the whole LLVM project) and analyze the results? http://clang.llvm.org/extra/clang-tidy/#running-clang-tidy-on-llvm describes the recommended way to run clang-tidy on LLVM.

[varjujan]

Yes, I did. The results from running the checker on LLVM are in the attached file. Sadly, I could'nt find any real mistakes but as I wrote in the summary, false positives can still indicate bad naming convention for some variables.

result.txt66 KBDownload

[alexfh]

In D20689#443266, @varjujan wrote:

Yes, I did. The results from running the checker on LLVM are in the attached file. Sadly, I could'nt find any real mistakes but as I wrote in the summary, false positives can still indicate bad naming convention for some variables.

result.txt66 KBDownload

It looks like the check doesn't meet the quality bar, at least, in its current form. "Bad naming convention" is a very arguable thing and I'm not sure we can claim that the pattern detected by the check is somehow an indication of a bad naming convention.

[varjujan]

I have implemented some more heuristics to achieve better results.

[varjujan]

I ran the check on multiple projects and tried to categorize the warnings: real errors, false positives, naming errors and coincidences. The results are attached. I got no warnings on LLVM.

postgres19 KBDownload

linuxKernel514 BDownload

xerces2 KBDownload

libreOffice487 BDownload

[alexfh]

In D20689#633889, @varjujan wrote:

I ran the check on multiple projects and tried to categorize the warnings: real errors, false positives, naming errors and coincidences. The results are attached. I got no warnings on LLVM.

I didn't find any "real errors" in the files you posted, which means that either all of these projects are of extreme code quality or that the error is rather unlikely to happen.

Another concern is that the check seems to treat argument types in a rather primitive way, e.g. it doesn't consider any type conversions or promotions.

[xazax.hun]

I think this might be better as a readability checker to find misleading variable or parameter names.

It would also be great to consider types. Unfortunately it probably means reimplementing some of the logic from Sema, since that information is not available at this point.

@varjujan
Do you actually use all of the heuristics that are implemented?

[varjujan]

@xazax.hun
Yes I do. Obviously some of them seem to be better than the others so I can remove a couple if needed.

[barancsuk]

Since the previous author, @varjujan is not available anymore, I take over the role of the author of this revision.

[barancsuk]

Major changes that have been made since the last update are as follows:

• The checker is moved from the module misc to readability
• It is checked, whether implicit type conversion is possible from the argument to the other parameter. The following conversion rules are considered:
  • Arithmetic type conversions
  • Pointer to pointer conversion (for multilevel pointers and also base/derived class pointers)
  • Array to pointer conversion
  • Function to function-pointer conversion
  • CV-qualifier compatibility is checked as well.

• The calculation of a heuristic`s result and the comparison of this result with the corresponding threshold value are performed by the same method, the heuristic`s function.
• The heuristic`s function is called only if the heuristic itself is turned on by the configuration settings.

Remark:
Implicit conversion rules of C are not checked separately, because, as I experienced when testing the checker on a larger code base, deeming all pointers convertible results in several false positives.

[barancsuk]

Check if argument and parameter numbers differ, add test cases for functions with default parameters

[barancsuk]

@alexfh, would you mind taking a look at the changes that have been introduced in the new patch?

The main improvements are:

• The checker has been shifted to the module readability.
• It is checked, whether implicit type conversion is possible from the argument to the parameter.

I have run the modified checker on some large code bases with the following results:
(The errors were categorized subjectively.)

• PostgreSQL: 32 renaming opportunities of 39 warnings
• Cpython: 10 renaming opportunities of 15 warnings
• Xerces: 6 renaming opportunities of 8 warnings
• FFmpeg: 5 renaming opportunities of 9 warnings
• OpenSSL: 3 renaming opportunities of 4 warnings
• LLVM: 20 renaming opportunities of 44 warnings

This article provides some evidence to support the feasibility of the checker as well.

icse2016-names.pdf426 KBDownload

The authors have proven that argument names are generally very similar to the corresponding parameters' names.
The presented empirical evidence also shows, that argument and parameter name dissimilarities are strong indicators of incorrect argument usages, or they identify renaming opportunities to improve code readability.
Moreover, the authors have even found 3 existing bugs in open source projects.

[alexfh]

In D20689#871947, @barancsuk wrote:

@alexfh, would you mind taking a look at the changes that have been introduced in the new patch?

The main improvements are:

• The checker has been shifted to the module readability.
• It is checked, whether implicit type conversion is possible from the argument to the parameter.

I have run the modified checker on some large code bases with the following results:
(The errors were categorized subjectively.)

• PostgreSQL: 32 renaming opportunities of 39 warnings
• Cpython: 10 renaming opportunities of 15 warnings
• Xerces: 6 renaming opportunities of 8 warnings
• FFmpeg: 5 renaming opportunities of 9 warnings
• OpenSSL: 3 renaming opportunities of 4 warnings
• LLVM: 20 renaming opportunities of 44 warnings

Is there a list of all the warnings? I'd like to take a closer look.

This article provides some evidence to support the feasibility of the checker as well.

icse2016-names.pdf426 KBDownload

The authors have proven that argument names are generally very similar to the corresponding parameters' names.
The presented empirical evidence also shows, that argument and parameter name dissimilarities are strong indicators of incorrect argument usages, or they identify renaming opportunities to improve code readability.
Moreover, the authors have even found 3 existing bugs in open source projects.

[barancsuk]

I attached the results of the tests.
The warnings are categorized into false positives and renaming opportunities.

PostgreSQL6 KBDownload

FFmpeg2 KBDownload

LLVM9 KBDownload

OpenSSL1 KBDownload

Xerces2 KBDownload

Cpython3 KBDownload

[barancsuk]

@alexfh, have you had a chance to look at the results yet?

[alexfh]

Sorry, I lost this patch. I've looked at the results and it still seems that the signal-to-noise ratio is quite low. There's definitely potential in using parameter name and argument spelling to detect possibly swapped arguments, and there's a recent research on this topic, where authors claim to have reached the true positive rate of 85% with decent recall. See https://research.google.com/pubs/pub46317.html. If you're interested in working on this check, I would suggest at least looking at the techniques described in that paper.

[whisperity]

I have developed a related check in D69560. That one considers types, but is an interface rule checker, and does not consider (any) potential call sites. Moreover, it does not consider "swaps" that happen across a function call, only, as the name implies, adjacent similar-type ranges.

Maybe one could lift the "is-similar-type", or rather, "is-accidentally-mixable-type" related ruling to some common location, and use type similarity as a precondition gate in the reports of this check?

docs/clang-tidy/checks/misc-redundant-expression.rst

18 ↗

(On Diff #113830)

This seems to be an unrelated diff.

test/clang-tidy/misc-redundant-expression.cpp

20 ↗

(On Diff #113830)

This entire file seems to be unrelated to the discussion at hand, perhaps a rebase went sideways?

[whisperity]

Assuming direct control. Previous colleagues and university mates departed for snowier pastures, time to try to do something with this check.

[whisperity]

First things first, we were 50 thousand (!) patches behind reality. Rebased to master. Fixed it to compile, too. Otherwise, NFC so far.

[whisperity]

Right, let's bump. I've checked the output of the checker on a set of test projects (our usual testbed, I should say) which contains around 15 projects, half of which was C and the other half C++. All projects were analysed independently in a virtual machine. All projects were analysed after checking out the latest release tag that was closest to Jan 2019. (I know this sounds dated, but the whole reproduction image was originally constructed for another checker I'm working on, and I did not wish to re-do the whole "What is needed to install this particular version?" pipeline.) The more important thing is that releases were analysed, which should, in theory, under-approximate the findings because releases are generally more polished than in-the-works code. The check was run as-is (sans a minor rebase issue that does not affect functionality) currently in the patch, namely, Diff 259508.

In total, I had received 194 reports (129 unique (1)). From this, I had found 8 (7 unique) true positives (marked Confirmed bug in CodeChecker terms), where something is visibly wrong with the call.
From this, there were 3 in a project where the called function's comment said that "The order of arguments <> and <> does not matter.", however, because this can never be figured out totally from the checker, I regarded the swapped call as a true positive. The fact that they had to seemingly create, inside the called function, some logic to detect and handle the swap shows that something was going wrong with the code's design for a long time.

In addition to these findings, I have identified 122 (75 unique) function call sites where the report about the potential swap (and the similarity to a different parameter name) is justifiable because the understandability of the code (especially to someone who is an outsider from virtually all of the projects analysed (2)) is hindered by the poor choice of argument or parameter names. The conclusions from these cases (marked Intentional in the CodeChecker database) are consistent with those drawn in [Pradel2013] and [Liu2016].

Now, onto the false positives. There were 64 (47 unique) cases. However, these cases can be further broken down into different categories, which I wasn't able to tally easily as CodeChecker only supports 3 unique categories, not infinite ones.

• Some of the false positives are what one would say "borderline": if the person reading the code reads it accurately, the reported "swap" does not fall into the understandability issue category. However, a famous case of this is from Postgres: reloid (IIRC, meaning relation owner id) and roleid (role (user) id) are the names of args/params of some functions. They are not swapped in the calls that exist in the code, but the similarity (swapping only 2 letters) makes it very easy to typo or misread the thing. In Postgres, there were 7 such cases.
• Approximately 5+5 cases are false positives but can be dealt with in heuristics. However, across the 17 projects, they do not account for a sizeable amount of cases.
  • Recursive function calls should be ignored. (This was mentioned in [Rice2017].)
  • Swapped and not-swapped calls appearing close to one another (i.e. constructs like b ? f(x,y) : f(y,x)) should be ignored too. This seems a bit harder to implement.
• There is a bug in the check's current implementation when calls from/to operator() is handled. (3) I'll look into fixing this.
• Binary operators should be ignored from reporting in general. Their parameters tend to have generic names, and the reports created from them is confusing.

Another generic observation is that the check's output is pretty wonky and hard to read at a glance, but this should be easy to fix. In addition, the observation of [Rice2017] about ignoring "patterned names" (i.e. arg1, arg2, ...) seems like a useful thing to add to the implementation, even though I had no findings at all where "ignoring patterned names" would've squelched a false positive report.

Agreeably, this check is limited compared to the previously linked [Rice2017], as it only checks the names in the call, not all variables in the surrounding context.

---

(1): I'm not exactly sure as to what "report uniqueing" in CodeChecker precisely does these days, but basically it uses the context of the bug and the checker message and whatnot to create a hash for the bug - "unique reports" mode shows each report belonging to the same hash only once.
(2): From the set of test projects, I only have some hands-on experience with parts of LLVM and Apache Xerces.
(3): Codes similar in nature to the following example of exact value forwarding to the call operator seems to still trigger the check. I have yet to actually pin down what causes this.

struct S
{
    int operator()(int a, int b, const char* c, double d);
};

struct T
{
    S* s;
    int operator(int a, int b, const char* c, double d)
    {
        return (*s)(a, b, c, d);
    }
};

[Pradel2013]: Michael Pradel, and Thomas R. Gross: Name-based analysis of equally typed method arguments. In: IEEE Transactions on Software Engineering, 39(8), pp. 1127-1143, 2013.
[Liu2016]: Hiu Liu, et al.: Nomen est Omen: Exploring and exploiting similarities between argument and parameter names. In: 38th IEEE International Conference on Software Engineering, pp. 1063-1073, 2016.
[Rice2017]: Andrew Rice, et al.: Detecting argument selection defects. In: Proceedings of the ACM on Programming Languages, 1, pp. 104:1-104:23, 2017.

[whisperity]

But of course, after having written all of that, I forgot to upload the results themselves... 😲

SuspiciousCall.sqlite32 MBDownload

[aaron.ballman]

In D20689#2457808, @whisperity wrote:

Right, let's bump.

Thank you for all of the detailed information on the performance of the check! I worked on a similar check in a recent past life and my intuition is that over a large corpus of code, this will still have quite a bit of false positives and false negatives. However, I think those can likely be handled by post-commit improvements like noticing abbreviations (def vs define) or synonyms (number vs numeral), being smarter about patterned names, etc. I think this check is worth moving forward with, but I'm not certain how @alexfh feels about it.

[whisperity]

(Typing this in here so it is not forgotten.) @aaron.ballman suggested in http://reviews.llvm.org/D69560#inline-893813 that a case of argument and parameter swaps could be discovered between forward declarations and the definitions, i.e.

void fn(int x, int y);

void fn(int y, int x) { ... } // Proparly mistakenly swapped names!

The conclusion was that this check (given this one does string distance, metrics, etc.) should be extended with such functionality. However, we need a better name for the check in that case!

[whisperity]

• Massively refactored and modernised the implementation
• Removed spaghetti code related to the check options and made their storage and defaults clearer
• Fixed modelling issues that caused false positives around lambdas, overloaded call operators, recursive calls, enumconstant arguments
• Made the abbreviation dictionary for the Abbreviations heuristic a check-option
• Made the check message much more legible
• Fixed a severe modelling issue about not respecting or handling record types passed by copy (CXXConstructExpr)
• Wrote the documentation for real, including all heuristics, default values, options, etc. The original documentation was basically empty. For the docs, I used @varjujan's thesis (which subject was this check), which is unfortunately only available in Hungarian, but it helped me understand what some of the enums and the Bound doing on the inside.
• Thanks to having written a proper documentation, I also renamed a few symbols and check options to better tell what they are about.

[aaron.ballman]

Pinging @alexfh for opinions about the check (especially any concerns about true or false positive rates). I continue to think this is a good check that's worth moving forward on.

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

32–33

signed char since we're doing > -1 below? Or better yet, int8_t because these aren't really characters?

52

We should probably document where all these numbers come from, but 66 definitely jumps out at me as being a bit strange. :-D

94

103–109

272–273

297

302–303

Can re-flow the comments.

342

Elsewhere we're using isPointerType() which is subtly different because it excludes ObjC object pointers. We should be consistent about the usage.

348

It seems like we're doing an awful lot of the same work as ASTContext::typesAreCompatible() and type compatibility rules are pretty complex, so I worry about this implementation being different than the ASTContext implementation. Have you explored whether we can reuse more of the logic from ASTContext here, or are they doing fundamentally different kinds of type compatibility checks?

463

468

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.h

39

The comment is somewhat confusing because the enumerators have the values 0 and 1, which are valid percentages but not likely what the comment means.

[whisperity]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

52

Unfortunately, I have absolutely no idea. All these values are percentages between 0 and 100 (-1 is just saying that "This heuristic doesn't accept percentages"), and this is written in the documentation now. However, the answer to "why 66%?", unless @varjujan can say something, I think is lost to history...

I'll read over his thesis once again, maybe I can find anything with regards to this.

Either way, I've detailed from both the code and the thesis how the percentages are meant. In some cases, the % is calculated as "% of the longer string's length". In the Leventhstein's case, it's actually inverted:

Dist = (1 - Dist / LongerLength) * 100;

So what this says is that if the current arg1-param1 arg2-param2 pairing has less than the inverse of 50% (which is more than 50%) of the longer string's edit distance, but the arg2-param1 and arg1-param2 (the suggested swapped order) has more than the inverse of 66% (which is less than 33%), then the swap will be suggested.

Originally these values were called LowerBound and UpperBound, respectively, which was saying even less about what they mean...

272–273

Good catch! I believe this is what happens when you write LaTeX and code at the same time? I didn't notice this when I was tidying up the code...

[whisperity]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

348

No, I didn't know that function even existed. This check must be older than that function.

[varjujan]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

52

Sadly, I think there isn't any scientific reason behind these numbers. They just looked ok after a couple of test runs. (Maybe they make sense for shorter arg names, like the 66 for 3 char long names.)

[whisperity]

NFC Made the code more legible, updated and clarified some comments, fixed grammar issues.

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

32–33

Oh right, I always forget char isn't guaranteed. int8_t seems like a better idea anyways (until we have int6_t...)

52

In [Rice2017], the inflexion point of the precision/recall plot is at around 0.55-ish threshold. They express this threshold as the distance of distances, i.e. 0 would mean the (a1, p1) - (a2, p2) pair is good as it is, and 1 would mean that it should definitely be (a2, p1) - (a1, p2) instead.

348

Actually, no, that function is pretty old... However, that function, and all the function it subsequently calls, require a non-const ASTContext. I have changed ASTContext:

╰─ git diff --cached --stat
 clang/include/clang/AST/ASTContext.h                                     | 67 ++++++++++++++++++++++++++++++++++++-------------------------------
 clang/lib/AST/ASTContext.cpp                                             | 72 +++++++++++++++++++++++++++++++++++++-----------------------------------
 2 files changed, 73 insertions(+), 66 deletions(-)

making related member functions and internal static functions take const ASTContext &/*.

This, by itself, did not break any of the tests of check-clang check-clang-unit check-clang-tools check-clang-extra-unit!

348–350

@aaron.ballman Changing the function to be

return Ctx.typesAreCompatible(ArgType, ParamType);

will make the checker miss the test case about T/const T& mixup.

void value_const_reference(int llllll, const int& kkkkkk);
void const_ref_value_swapped() {
  const int& kkkkkk = 42;
  const int& llllll = 42;
  value_const_reference(kkkkkk, llllll);
  // error: CHECK-MESSAGES: expected string not found in input:
  // warning: 1st argument 'kkkkkk' (passed to 'llllll') looks like it might be swapped with the 2nd, 'llllll' (passed to 'kkkkkk')
}

---

Setting bool CompareUnqualified = true (3rd argument to typesAreCompatible) doesn't help either.
Which is valid from typesAreCompatible's perspective... that function answer the question, applied to the context of the above test: "Is llllll = kkkkkk; valid?", which is obviously false as both are const T&s.

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.h

39

I'll rename it to BoundKind and update the comments.

[aaron.ballman]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

52

Thank you for the explanations! I'm fine with the values as they are (they're defaults that can be changed anyway).

165

348

Doubtful -- typesAreCompatible() is critical for checking the semantics of assignment in C, overloading in C++, etc. It may have simply been overlooked when writing this check. Given the complexities of type checking, my intuition is that we should be leaning on ASTContext for as much of this functionality as we can get away with. That will also get us nice "extras" like caring about address spaces, ARC, etc which are what got me worried when I started looking at this implementation.

625

TIL about %ordinal in diagnostics, thanks for that! :-D

633–635

645

653–657

658–661

Can this case happen?

708–709

Should the length of what's considered "too short" be a configuration option? I think 3 is a good default.

740

Otherwise it looks like we could read an uninitalized value later (if GotBound was None).

740–742

There's some type confusion going on here between unsigned char and char -- I think all of these uses should switch to uint8_t or int8_t (or possibly just int given that these values all wind up being promoted to int anyway).

744–745

Any reason not to move this below the switch and use = instead of |= within the cases? (Or return from the cases directly?)

clang-tools-extra/docs/clang-tidy/checks/readability-suspicious-call-argument.rst

62–69

I wonder how Hungarian notation impacts this heuristic -- I would imagine a lot of similar prefixes in such a code base, and things like lpstr as a prefix could be a pretty large chunk of some identifiers.

76

Similar to above, I wonder how numeric digits impact this heuristic -- do the defaults consider this to be a swap?

void foo(int frobble1, int frobble2);
foo(frobble2, frobble1); // Hopefully identified as a swap
foo(bar2, bar1); // How about this?

[whisperity]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

658–661

Oops... It seems I posted the updated patch right where you were writing more comments and we got into a data race. Which case are you referring to? It's now affixed to a diag( call for me...

clang-tools-extra/docs/clang-tidy/checks/readability-suspicious-call-argument.rst

62–69

The switch is only warned if it would be type-safe. If the HN prefix is in both the same way, then it could be ignored. Thus, given f(const char* lpszFoo, const char* lpszBar, uint16_t psnzXXX) {}, if I do a f(lpszX, lpszA, ...);, it should consider in both cases that the prefix is common and matches. Note that to produce a diagnostic, two things has to be proven: first, that the current ordering is dissimilar (below threshold A), and second, that the potential swapped ordering is more similar (above threshold B).

76

Currently, neither of these are matched. I have to look into why the first isn't... it really should, based on the "equality" heuristic. It's too trivial.

The second... well... that's trickier. I would say it shouldn't match, because if it did, we would be swamped with false positives. The suffix is only 1 character, and we need 25/30% based on the string's length.

[aaron.ballman]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

348–350

Yeah, I expect there to be a delta between the work this check is doing and the existing work done by typesAreCompatible(). However, given the complexity of type compatibility checking, I'd say it's better for us to try to refactor the ASTContext functionality so that we can share as much of the implementation as plausible rather than duplicate some really difficult logic in the tidy check.

658–661

Hehe, it's "fun" when the comments move around like this, isn't it? :-D I meant the else clause in:

for (std::size_t I = 0, E = CalleeFuncDecl->getNumParams(); I != E; ++I) {
  if (const ParmVarDecl *Param = CalleeFuncDecl->getParamDecl(I)) {
    ParamTypes.push_back(Param->getType());

    if (IdentifierInfo *II = Param->getIdentifier()) {
      ParamNames.push_back(II->getName());
    } else {
      ParamNames.push_back(StringRef());
    }
  } else { // This seems like it should be impossible, no?
    ParamTypes.push_back(QualType());
    ParamNames.push_back(StringRef());
  }
}

clang-tools-extra/docs/clang-tidy/checks/readability-suspicious-call-argument.rst

76

I agree that the first one should be caught and I also agree that the second one is tricky but that matching it would likely increase false positives.

[whisperity]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

658–661

Oh, nevermind, there is a button that shows me the older diff where it's aligned properly.

And yeah, it seems it can't, getParamDecl always returns a ParmVarDecl. Weird issues might arise when the vectors that are built here get out of sync (such as the issue we had with operator() calls before I fixed it!), so I understood the reason behind keeping the two functions parallel with each other in terms of pure visuals, even.

744–745

Returning from the cases directly is a bad idea because we want to try all heuristics and only say false if none of them matches.
But this break in a very bad location, I agree.

[whisperity]

• NFC Fixed some nits
• Added a new check option, MinimumIdentifierNameLength instead of a hardcoded 3 value. Defaults to 3.
• Fixed an issue with heuristics matching only in one direction but not in the other direction silenced a warning that clearly should have been there.

[whisperity]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

348

I talked with @aaron.ballman in private about this a little, and unfortunately, the route to call typesAreCompatible() is no dice. First things first, in C++ mode, that function just early returns (essentially) == on the type. Otherwise, it would call QualType ASTContext::mergeTypes(QualType, QualType, ...). Okay, let's delve into this thing. Now, first things first, somewhere down the line if you happen to give it something C++-specific (e.g. LValueReferenceType), it will just assert into your face. (Because C++ things are only compatible if the types are equal, most likely. In C++ mode, mergeType shouldn't be, and doesn't get, called.)

But there are even more issues with this function. Put simply, it does not perform any "implicit conversions". Sic!, I'm putting it in between quotes, but the fact is that if one type is const and the other isn't, it will just say "Nope, these are not compatible.".
So modelling the whole idea of "is passing an argument of type T acceptable to a parameter of type U?" is not possible with these functions.

So even though typesAreCompatible()'s documentation comment says:

Compatibility predicates used to check assignment expressions.

by far and large, we are not having the right kinds of types in our hands (in terms of this check) to fall back to this logic.

I've done a skim of the codebase to find call sites for these functions and try to explore my way back and forth from there to see how this function should be used, and the results are daunting.

The image is from lib/Sema/SemaExpr.cpp somewhere along the lines of checking address spaces (for Obj-C, the vast majority of this function seems to only deal with Obj-C-specific things!), from checkConditionalPointerCompatibility. The call to mergeTypes() is in the bottom left, the right pane is the continuation below on the result here is used.

It seems to me (and many other call sites to this function tend to behave the same way) that Sema does a lot of casting-away-things (like constness) before it can reasonably call this function, and if the result comes back valid, this function (and many others) then just re-apply manually the things that were originally cast off.

Thus, I see that if we would call this function in the check, a lot (if not all, or maybe even more than now!) of the logic that is currently here would still have to remain to prefix and suffix the call to mergeTypes().

Just for the sake of trying it all out, I put together a version where I leave the handling of the "C++ stuff" (like references) to the checker, but the rest back to the ASTContext, but it didn't fall into place:

ParamType = PointerType 0x5627819d0930 'const int *'
`-QualType 0x5627819cfef1 'const int' const
  `-BuiltinType 0x5627819cfef0 'int'
ArgType = PointerType 0x562781a49290 'int *'
`-BuiltinType 0x5627819cfef0 'int'
mergeType(Param, Arg) = <<<NULL>>>
mergeType(Arg, Param) = <<<NULL>>>

348

╰─ git diff --cached --stat
 clang/include/clang/AST/ASTContext.h                                     | 67 ++++++++++++++++++++++++++++++++++++-------------------------------
 clang/lib/AST/ASTContext.cpp                                             | 72 +++++++++++++++++++++++++++++++++++++-----------------------------------
 2 files changed, 73 insertions(+), 66 deletions(-)

ASTContext_typesAreCompatible_const.patch17 KBDownload

[whisperity]

Rebase over D98635. Highlighting only the parameter's name, not the entire range of the parameter.

[whisperity]

@aaron.ballman @alexfh Bump! How shall we move further?

[whisperity]

• NFC Fix lint of header guard nomenclature
• NFC Tear out obsolete LLVM_DEBUG calls from the implementation

[whisperity]

NFC Fix the header guard name lint again, hopefully to the proper value this time. I forgot the CHECK out of it...

[whisperity]

NFC Rebase and fix things broken by D104819.

[aaron.ballman]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

186

Should this be a case insensitive comparison?

208

269

300

372

404

409–410

Should this move down closer to where it's used?

426–428

438

454

468

It would be good to have a test case involving private inheritance.

580

589

695–696

Range-based for loop over CalledFuncDecl->params()?

802

This looks pretty reachable to me in the case where there's no bound.

[whisperity]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

409–410

Sure!

802

I'm not sure if that is the case. I added the llvm_unreachable so we don't get a warning about the function not having a return value on every code path. The switch covers all potential heuristics that are in the check right now, but if we add a new heuristic (to the enum) and forget to write it in, we will get a -Wswitch warning here. A default case doesn't apply here, further developers should be encouraged to wire new heuristics in properly.

[whisperity]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

695–696

(Note: it's parameters() now, not params()!)

[whisperity]

• Fix comments and code according to comments
• Turned Substring into a case-insensitive heuristic.
• Added some missing tests.

[whisperity]

NFC Fix ReleaseNotes entry getting out of place during rebases earlier.

[aaron.ballman]

I think this looks good to me. @alexfh, you had raised questions about this meeting the quality bar. I think those concerns are valid in the abstract (swapped argument checking requires heuristics), but I'd argue that most existing code bases that actually have swapped args finds those bugs in production, so the rate of true positives on existing code is often small. I see the value from this check coming from checking new code as it's written, as part of a CI pipeline for instance. With that in mind, do you still have concerns about this check?

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

802

Oops, ignore my think-o! I somehow read the switch as switching over Threshold and not H and confused myself.

[whisperity]

Bump. 🙂 I would prefer this check to be able to go into Clang-Tidy 13 if it's okay, together with the other check I implemented.

[aaron.ballman]

In D20689#2874473, @whisperity wrote:

Bump. 🙂 I would prefer this check to be able to go into Clang-Tidy 13 if it's okay, together with the other check I implemented.

@alexfh has had a few weeks to respond, so I'm going to give my explicit LG just in case he isn't available to perform a review. Please give Alex until the end of the week to comment on here before landing, but given the considerable review and discussion on this already, I think we can address remaining concerns post-commit if he's unavailable for the moment.

clang-tools-extra/clang-tidy/readability/CMakeLists.txt

add_clang_library(clangTidyReadabilityModule
   RedundantSmartptrGetCheck.cpp
   RedundantStringCStrCheck.cpp
   RedundantStringInitCheck.cpp
   SimplifyBooleanExprCheck.cpp
   SimplifySubscriptExprCheck.cpp
   StaticAccessedThroughInstanceCheck.cpp
   StaticDefinitionInAnonymousNamespaceCheck.cpp
   StringCompareCheck.cpp
+  SuspiciousCallArgumentCheck.cpp
   UniqueptrDeleteReleaseCheck.cpp
   UppercaseLiteralSuffixCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTidyUtils
   clangTooling
   )

clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp

 #include "RedundantSmartptrGetCheck.h"
 #include "RedundantStringCStrCheck.h"
 #include "RedundantStringInitCheck.h"
 #include "SimplifyBooleanExprCheck.h"
 #include "SimplifySubscriptExprCheck.h"
 #include "StaticAccessedThroughInstanceCheck.h"
 #include "StaticDefinitionInAnonymousNamespaceCheck.h"
 #include "StringCompareCheck.h"
+#include "SuspiciousCallArgumentCheck.h"
 #include "UniqueptrDeleteReleaseCheck.h"
 #include "UppercaseLiteralSuffixCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace readability {
 
 class ReadabilityModule : public ClangTidyModule {
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<RedundantSmartptrGetCheck>(
         "readability-redundant-smartptr-get");
     CheckFactories.registerCheck<RedundantStringCStrCheck>(
         "readability-redundant-string-cstr");
     CheckFactories.registerCheck<RedundantStringInitCheck>(
         "readability-redundant-string-init");
     CheckFactories.registerCheck<SimplifyBooleanExprCheck>(
         "readability-simplify-boolean-expr");
+    CheckFactories.registerCheck<SuspiciousCallArgumentCheck>(
+        "readability-suspicious-call-argument");
     CheckFactories.registerCheck<UniqueptrDeleteReleaseCheck>(
         "readability-uniqueptr-delete-release");
     CheckFactories.registerCheck<UppercaseLiteralSuffixCheck>(
         "readability-uppercase-literal-suffix");
   }
 };
 
 // Register the ReadabilityModule using this statically initialized variable.

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.h

+//===--- SuspiciousCallArgumentCheck.h - clang-tidy--------------*- C -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SUSPICIOUS_CALL_ARGUMENT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SUSPICIOUS_CALL_ARGUMENT_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// This checker finds those function calls where the function arguments are
+/// provided in an incorrect order. It compares the name of the given variable
+/// to the argument name in the function definition.
+/// It issues a message if the given variable name is similar to an another
+/// function argument in a function call. It uses case insensitive comparison.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-suspicious-call-argument.html
+class SuspiciousCallArgumentCheck : public ClangTidyCheck {
+public:
+  SuspiciousCallArgumentCheck(StringRef Name, ClangTidyContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+  const static unsigned VectorSmallSize = 8;
+
+private:
+  enum class Heuristic {
+    Equality,
+    Abbreviation,
+    Prefix,
+    Suffix,
+    Levenshtein,
+    Substring,
+    JaroWinkler,
+    Dice
+  };
+
+  enum class Bound { Lower, Upper };
+
+  SmallVector<Heuristic, VectorSmallSize> AppliedHeuristics;
+
+  SmallVector<QualType, VectorSmallSize> ArgTypes;
+  SmallVector<StringRef, VectorSmallSize> ArgNames;
+  SmallVector<QualType, VectorSmallSize> ParamTypes;
+  SmallVector<StringRef, VectorSmallSize> ParamNames;
+
+  const bool Equality;
+  const bool Abbreviation;
+  const bool Levenshtein;
+  const bool Prefix;
+  const bool Suffix;
+  const bool Substring;
+  const bool JaroWinkler;
+  const bool Dice;
+
+  const int LevenshteinUpperBound;
+  const int PrefixUpperBound;
+  const int SuffixUpperBound;
+  const int SubstringUpperBound;
+  const int JaroWinklerUpperBound;
+  const int DiceUpperBound;
+
+  const int LevenshteinLowerBound;
+  const int PrefixLowerBound;
+  const int SuffixLowerBound;
+  const int SubstringLowerBound;
+  const int JaroWinklerLowerBound;
+  const int DiceLowerBound;
+
+  void setParamNamesAndTypes(const FunctionDecl *CalleeFuncDecl);
+
+  void setArgNamesAndTypes(const CallExpr *MatchedCallExpr,
+                           unsigned InitialArgIndex);
+
+  bool areParamAndArgComparable(unsigned Position1, unsigned Position2,
+                                const ASTContext *Ctx) const;
+
+  bool areArgsSwapped(unsigned Position1, unsigned Position2) const;
+
+  bool areNamesSimilar(StringRef Arg, StringRef Param, Bound bound) const;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SUSPICIOUS_CALL_ARGUMENT_H

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

+//===--- SuspiciousCallArgumentCheck.cpp - clang-tidy----------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "SuspiciousCallArgumentCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Type.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <sstream>
+
+using namespace clang::ast_matchers;
+
+namespace {
+
+llvm::StringMap<llvm::StringRef> AbbreviationDictionary;
+
+} // namespace
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+static bool applyEqualityHeuristic(StringRef Arg, StringRef Param) {
+  return Arg.equals_lower(Param);
+}
+
+static bool applyAbbreviationHeuristic(StringRef Arg, StringRef Param) {
+  if (AbbreviationDictionary.find(Arg) != AbbreviationDictionary.end()) {
+    if (Param.compare(AbbreviationDictionary.lookup(Arg)) == 0)
+      return true;
+  }
+
+  if (AbbreviationDictionary.find(Param) != AbbreviationDictionary.end()) {
+    if (Arg.compare(AbbreviationDictionary.lookup(Param)) == 0)
+      return true;
+  }
+
+  return false;
+}
+
+// Check whether the shorter String is a prefix of the longer String.
+static bool applyPrefixHeuristic(StringRef Arg, StringRef Param,
+                                 unsigned threshold) {
+  StringRef Shorter = Arg.size() < Param.size() ? Arg : Param;
+  StringRef Longer = Arg.size() >= Param.size() ? Arg : Param;
+
+  double PrefixMatch = 0;
+  if (Longer.startswith_lower(Shorter))
+    PrefixMatch = (double)Shorter.size() / Longer.size() * 100;
+
+  return PrefixMatch > threshold;
+}
+
+// Check whether the shorter String is a suffix of the longer String.
+static bool applySuffixHeuristic(StringRef Arg, StringRef Param,
+                                 unsigned threshold) {
+  StringRef Shorter = Arg.size() < Param.size() ? Arg : Param;
+  StringRef Longer = Arg.size() >= Param.size() ? Arg : Param;
+
+  double SuffixMatch = 0;
+  if (Longer.endswith_lower(Shorter))
+    SuffixMatch = (double)Shorter.size() / Longer.size() * 100;
+
+  return SuffixMatch > threshold;
+}
+
+static bool applySubstringHeuristic(StringRef Arg, StringRef Param,
+                                    unsigned threshold) {
+  unsigned MaxLength = 0;
+  llvm::SmallVector<unsigned, SuspiciousCallArgumentCheck::VectorSmallSize>
+      Current(Param.size());
+  llvm::SmallVector<unsigned, SuspiciousCallArgumentCheck::VectorSmallSize>
+      Previous(Param.size());
+
+  for (unsigned i = 0; i < Arg.size(); ++i) {
+    for (unsigned j = 0; j < Param.size(); ++j) {
+      if (Arg[i] == Param[j]) {
+        if (i == 0 || j == 0) {
+          Current[j] = 1;
+        } else {
+          Current[j] = 1 + Previous[j - 1];
+        }
+
+        MaxLength = std::max(MaxLength, Current[j]);
+      } else {
+        Current[j] = 0;
+      }
+    }
+
+    Current.swap(Previous);
+  }
+
+  size_t LongerLength = std::max(Arg.size(), Param.size());
+  double SubstringRatio = (double)MaxLength / LongerLength * 100;
+
+  return SubstringRatio > threshold;
+}
+
+static bool applyLevenshteinHeuristic(StringRef Arg, StringRef Param,
+                                      unsigned threshold) {
+  unsigned Dist = Arg.edit_distance(Param);
+  size_t LongerLength = std::max(Arg.size(), Param.size());
+  Dist = (1 - (double)Dist / LongerLength) * 100;
+  return Dist > threshold;
+}
+
+// https://en.wikipedia.org/wiki/Jaro%E2%80%93Winkler_distance
+static bool applyJaroWinklerHeuristic(StringRef Arg, StringRef Param,
+                                      unsigned threshold) {
+  int Match = 0, Transpos = 0;
+  int ArgLen = Arg.size();
+  int ParamLen = Param.size();
+  llvm::SmallVector<int, SuspiciousCallArgumentCheck::VectorSmallSize> ArgFlags(
+      ArgLen);
+  llvm::SmallVector<int, SuspiciousCallArgumentCheck::VectorSmallSize>
+      ParamFlags(ParamLen);
+  int Range = std::max(0, std::max(ArgLen, ParamLen) / 2 - 1);
+
+  // Calculate matching characters.
+  for (int i = 0; i < ParamLen; i++) {
+    for (int j = std::max(i - Range, 0), l = std::min(i + Range + 1, ArgLen);
+         j < l; j++) {
+      if (tolower(Param[i]) == tolower(Arg[j]) && !ArgFlags[j]) {
+        ArgFlags[j] = 1;
+        ParamFlags[i] = 1;
+        Match++;
+        break;
+      }
+    }
+  }
+
+  if (!Match)
+    return false;
+
+  // Calculate character transpositions.
+  int l = 0;
+  for (int i = 0; i < ParamLen; i++) {
+    if (ParamFlags[i] == 1) {
+      int j;
+      for (j = l; j < ArgLen; j++) {
+        if (ArgFlags[j] == 1) {
+          l = j + 1;
+          break;
+        }
+      }
+      if (tolower(Param[i]) != tolower(Arg[j]))
+        Transpos++;
+    }
+  }
+  Transpos /= 2;
+
+  // Jaro distance.
+  double Dist = (((double)Match / ArgLen) + ((double)Match / ParamLen) +
+                 ((double)(Match - Transpos) / Match)) /
+                3.0;
+
+  // Calculate common string prefix up to 4 chars.
+  l = 0;
+  for (int i = 0; i < std::min(std::min(ArgLen, ParamLen), 4); i++)
+    if (tolower(Arg[i]) == tolower(Param[i]))
+      l++;
+
+  // Jaro-Winkler distance.
+  Dist = (Dist + (l * 0.1 * (1 - Dist))) * 100;
+
+  return Dist > threshold;
+}
+
+// https://en.wikipedia.org/wiki/S%C3%B8rensen%E2%80%93Dice_coefficient
+static bool applyDiceHeuristic(StringRef Arg, StringRef Param,
+                               unsigned threshold) {
+  double Dice = 0;
+
+  llvm::StringSet<> Arg_bigrams;
+  llvm::StringSet<> Param_bigrams;
+
+  // Extract character bigrams from Arg.
+  for (unsigned i = 0; i < (Arg.size() - 1); i++) {
+    Arg_bigrams.insert(Arg.substr(i, 2));
+  }
+
+  // Extract character bigrams from Param.
+  for (unsigned i = 0; i < (Param.size() - 1); i++) {
+    Param_bigrams.insert(Param.substr(i, 2));
+  }
+
+  int Intersection = 0;
+
+  // Find the intersection between the two sets.
+  for (auto IT = Param_bigrams.begin(); IT != Param_bigrams.end(); IT++) {
+    Intersection += Arg_bigrams.count((IT->getKey()));
+  }
+
+  // Calculate dice coefficient.
+  Dice =
+      (Intersection * 2.0) / (Arg_bigrams.size() + Param_bigrams.size()) * 100;
+
+  return Dice > threshold;
+}
+
+// Checks if ArgType binds to ParamType ragerding reference-ness and
+// cv-qualifiers.
+static bool areRefAndQualCompatible(QualType ArgType, QualType ParamType) {
+  return !ParamType->isReferenceType() ||
+         ParamType.getNonReferenceType().isAtLeastAsQualifiedAs(
+             ArgType.getNonReferenceType());
+}
+
+static bool isPointerOrArray(const QualType &TypeToCheck) {
+  return TypeToCheck->isAnyPointerType() || TypeToCheck->isArrayType();
+}
+
+// Checks whether ArgType is an array type identical to ParamType`s array type.
+// Enforces array elements` qualifier compatibility as well.
+static bool isCompatibleWithArrayReference(const QualType &ArgType,
+                                           const QualType &ParamType) {
+  if (!ArgType->isArrayType())
+    return false;
+  // Here, qualifiers belong to the elements of the arrays.
+  if (!ParamType.isAtLeastAsQualifiedAs(ArgType))
+    return false;
+
+  return ParamType.getUnqualifiedType() == ArgType.getUnqualifiedType();
+}
+
+static void convertToPointeeOrArrayElementQualType(QualType &TypeToConvert) {
+  unsigned CVRqualifiers = 0;
+  // Save array element qualifiers, since getElementType() removes qualifiers
+  // from array elements.
+  if (TypeToConvert->isArrayType())
+    CVRqualifiers = TypeToConvert.getLocalQualifiers().getCVRQualifiers();
+  TypeToConvert = TypeToConvert->isPointerType()
+                      ? TypeToConvert->getPointeeType()
+                      : TypeToConvert->getAsArrayTypeUnsafe()->getElementType();
+  TypeToConvert = TypeToConvert.withCVRQualifiers(CVRqualifiers);
+}
+
+// Checks if multilevel pointers` qualifiers compatibility continues on the
+// current pointer evel.
+// For multilevel pointers, C++ permits conversion, if every cv-qualifier in
+// ArgType also appears in the corresponding position in ParamType,
+// and if PramType has a cv-qualifier that's not in ArgType, then every * in
+// ParamType to the right
+// of that cv-qualifier, except the last one, must also be const-qualified.
+static bool arePointersStillQualCompatible(QualType ArgType, QualType ParamType,
+                                           bool &IsParamContinuouslyConst) {
+  // The types are compatible, if the parameter is at least as qualified as the
+  // argument, and if it is more qualified, it has to be const on upper pointer
+  // levels.
+  bool AreTypesQualCompatible =
+      ParamType.isAtLeastAsQualifiedAs(ArgType) &&
+      (!ParamType.hasQualifiers() || IsParamContinuouslyConst);
+  // Check whether the parameter's constness continues at the current pointer
+  // level.
+  IsParamContinuouslyConst &= ParamType.isConstQualified();
+
+  return AreTypesQualCompatible;
+}
+
+// Checks whether multilevel pointers are compatible in terms of levels,
+// qualifiers and pointee type.
+static bool arePointerTypesCompatible(QualType ArgType, QualType ParamType,
+                                      bool IsParamContinuouslyConst) {
+
+  if (!arePointersStillQualCompatible(ArgType, ParamType,
+                                      IsParamContinuouslyConst))
+    return false;
+
+  do {
+    // Step down one pointer level.
+    convertToPointeeOrArrayElementQualType(ArgType);
+    convertToPointeeOrArrayElementQualType(ParamType);
+
+    // Check whether cv-qualifiers premit compatibility on
+    // current level.
+    if (!arePointersStillQualCompatible(ArgType, ParamType,
+                                        IsParamContinuouslyConst))
+      return false;
+
+    if (ParamType.getUnqualifiedType() == ArgType.getUnqualifiedType())
+      return true;
+
+  } while (ParamType->isAnyPointerType() && ArgType->isAnyPointerType());
+  // The final type does not match, or pointer levels differ.
+  return false;
+}
+
+// Checks whether ArgType converts implicitly to ParamType.
+static bool areTypesCompatible(QualType ArgType, QualType ParamType,
+                               const ASTContext *Ctx) {
+  if (ArgType.isNull() || ParamType.isNull())
+    return false;
+
+  ArgType = ArgType.getCanonicalType();
+  ParamType = ParamType.getCanonicalType();
+
+  if (ArgType == ParamType)
+    return true;
+
+  // Check for constness and reference compatibility.
+  if (!areRefAndQualCompatible(ArgType, ParamType))
+    return false;
+
+  bool IsParamReference = ParamType->isReferenceType();
+
+  // Reference-ness has already been checked ad should be removed
+  // before further checking.
+  ArgType = ArgType.getNonReferenceType();
+  ParamType = ParamType.getNonReferenceType();
+
+  bool IsParamContinuouslyConst =
+      !IsParamReference || ParamType.getNonReferenceType().isConstQualified();
+
+  if (ParamType.getUnqualifiedType() == ArgType.getUnqualifiedType())
+    return true;
+
+  // Arithmetic types are interconvertible, except scoped enums.
+  if (ParamType->isArithmeticType() && ArgType->isArithmeticType()) {
+    if ((ParamType->isEnumeralType() &&
+         ParamType->getAs<EnumType>()->getDecl()->isScoped()) ||
+        (ArgType->isEnumeralType() &&
+         ArgType->getAs<EnumType>()->getDecl()->isScoped()))
+      return false;
+
+    return true;
+  }
+
+  // Check if the argument and the param are both function types (the parameter
+  // decayed to
+  // a function pointer).
+  if (ArgType->isFunctionType() && ParamType->isFunctionPointerType()) {
+    ParamType = ParamType->getPointeeType();
+    return ArgType == ParamType;
+  }
+
+  // Arrays or pointer arguments convert to array or pointer parameters.
+  if (!(isPointerOrArray(ArgType) && isPointerOrArray(ParamType)))
+    return false;
+
+  // When ParamType is an array reference, ArgType has to be of the same sized,
+  // array type with cv-compatible elements.
+  if (IsParamReference && ParamType->isArrayType())
+    return isCompatibleWithArrayReference(ArgType, ParamType);
+
+  // Remove the first level of indirection.
+  convertToPointeeOrArrayElementQualType(ArgType);
+  convertToPointeeOrArrayElementQualType(ParamType);
+
+  // Check qualifier compatibility on the next level.
+  if (!ParamType.isAtLeastAsQualifiedAs(ArgType))
+    return false;
+
+  if (ParamType.getUnqualifiedType() == ArgType.getUnqualifiedType())
+    return true;
+
+  // At this point, all possible C language implicit conversion were checked
+  if (!Ctx->getLangOpts().CPlusPlus)
+    return false;
+
+  // Check whether ParamType and ArgType were both pointers to a class or a
+  // struct, and check for inheritance.
+  if (ParamType->isStructureOrClassType() &&
+      ArgType->isStructureOrClassType()) {
+    auto *ArgDecl = ArgType->getAsCXXRecordDecl();
+    auto *ParamDecl = ParamType->getAsCXXRecordDecl();
+    if (!ArgDecl || !ArgDecl->hasDefinition() || !ParamDecl ||
+        !ParamDecl->hasDefinition())
+      return false;
+
+    return ArgDecl->isDerivedFrom(ParamDecl);
+  }
+
+  // Unless argument and param are both multilevel pointers, the types are not
+  // convertible.
+  if (!(ParamType->isAnyPointerType() && ArgType->isAnyPointerType()))
+    return false;
+
+  return arePointerTypesCompatible(ArgType, ParamType,
+                                   IsParamContinuouslyConst);
+}
+
+// Constructor.
+SuspiciousCallArgumentCheck::SuspiciousCallArgumentCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context), Equality(Options.get("Equality", true)),
+      Abbreviation(Options.get("Abbreviation", true)),
+      Levenshtein(Options.get("Levenshtein", true)),
+      Prefix(Options.get("Prefix", true)), Suffix(Options.get("Suffix", true)),
+      Substring(Options.get("Substring", true)),
+      JaroWinkler(Options.get("JaroWinkler", true)),
+      Dice(Options.get("Dice", true)),
+      LevenshteinUpperBound(Options.get("LevenshteinUpperBound", 66)),
+      PrefixUpperBound(Options.get("PrefixUpperBound", 30)),
+      SuffixUpperBound(Options.get("SuffixUpperBound", 30)),
+      SubstringUpperBound(Options.get("SubstringUpperBound", 50)),
+      JaroWinklerUpperBound(Options.get("JaroWinklerUpperBound", 85)),
+      DiceUpperBound(Options.get("DiceUpperBound", 70)),
+      LevenshteinLowerBound(Options.get("LevenshteinLowerBound", 50)),
+      PrefixLowerBound(Options.get("PrefixLowerBound", 25)),
+      SuffixLowerBound(Options.get("SuffixLowerBound", 25)),
+      SubstringLowerBound(Options.get("SubstringLowerBound", 40)),
+      JaroWinklerLowerBound(Options.get("JaroWinklerLowerBound", 75)),
+      DiceLowerBound(Options.get("DiceLowerBound", 60)) {
+
+  AbbreviationDictionary.insert(std::make_pair("ptr", "pointer"));
+  AbbreviationDictionary.insert(std::make_pair("len", "length"));
+  AbbreviationDictionary.insert(std::make_pair("addr", "address"));
+  AbbreviationDictionary.insert(std::make_pair("arr", "array"));
+  AbbreviationDictionary.insert(std::make_pair("cpy", "copy"));
+  AbbreviationDictionary.insert(std::make_pair("src", "source"));
+  AbbreviationDictionary.insert(std::make_pair("val", "value"));
+  AbbreviationDictionary.insert(std::make_pair("ln", "line"));
+  AbbreviationDictionary.insert(std::make_pair("col", "column"));
+  AbbreviationDictionary.insert(std::make_pair("num", "number"));
+  AbbreviationDictionary.insert(std::make_pair("nr", "number"));
+  AbbreviationDictionary.insert(std::make_pair("stmt", "statement"));
+  AbbreviationDictionary.insert(std::make_pair("var", "variable"));
+  AbbreviationDictionary.insert(std::make_pair("vec", "vector"));
+  AbbreviationDictionary.insert(std::make_pair("buf", "buffer"));
+  AbbreviationDictionary.insert(std::make_pair("txt", "text"));
+  AbbreviationDictionary.insert(std::make_pair("dest", "destination"));
+  AbbreviationDictionary.insert(std::make_pair("elem", "element"));
+  AbbreviationDictionary.insert(std::make_pair("wdth", "width"));
+  AbbreviationDictionary.insert(std::make_pair("hght", "height"));
+  AbbreviationDictionary.insert(std::make_pair("cnt", "count"));
+  AbbreviationDictionary.insert(std::make_pair("i", "index"));
+  AbbreviationDictionary.insert(std::make_pair("idx", "index"));
+  AbbreviationDictionary.insert(std::make_pair("ind", "index"));
+  AbbreviationDictionary.insert(std::make_pair("attr", "atttribute"));
+  AbbreviationDictionary.insert(std::make_pair("pos", "position"));
+  AbbreviationDictionary.insert(std::make_pair("lst", "list"));
+  AbbreviationDictionary.insert(std::make_pair("str", "string"));
+  AbbreviationDictionary.insert(std::make_pair("srvr", "server"));
+  AbbreviationDictionary.insert(std::make_pair("clnt", "client"));
+  AbbreviationDictionary.insert(std::make_pair("ref", "reference"));
+  AbbreviationDictionary.insert(std::make_pair("dst", "distance"));
+  AbbreviationDictionary.insert(std::make_pair("dist", "distance"));
+
+  if (Equality)
+    AppliedHeuristics.push_back(Heuristic::Equality);
+  if (Abbreviation)
+    AppliedHeuristics.push_back(Heuristic::Abbreviation);
+  if (Levenshtein)
+    AppliedHeuristics.push_back(Heuristic::Levenshtein);
+  if (Prefix)
+    AppliedHeuristics.push_back(Heuristic::Prefix);
+  if (Suffix)
+    AppliedHeuristics.push_back(Heuristic::Suffix);
+  if (Substring)
+    AppliedHeuristics.push_back(Heuristic::Substring);
+  if (JaroWinkler)
+    AppliedHeuristics.push_back(Heuristic::JaroWinkler);
+  if (Dice)
+    AppliedHeuristics.push_back(Heuristic::Dice);
+}
+
+// Options.
+void SuspiciousCallArgumentCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+
+  Options.store(Opts, "Equality", Equality);
+  Options.store(Opts, "Abbreviation", Abbreviation);
+  Options.store(Opts, "Levenshtein", Levenshtein);
+  Options.store(Opts, "Prefix", Prefix);
+  Options.store(Opts, "Suffix", Suffix);
+  Options.store(Opts, "Substring", Substring);
+  Options.store(Opts, "JaroWinkler", JaroWinkler);
+  Options.store(Opts, "Dice", Dice);
+
+  Options.store(Opts, "LevenshteinUpperBound", LevenshteinUpperBound);
+  Options.store(Opts, "PrefixUpperBound", PrefixUpperBound);
+  Options.store(Opts, "SuffixUpperBound", SuffixUpperBound);
+  Options.store(Opts, "SubstringUpperBound", SubstringUpperBound);
+  Options.store(Opts, "JaroWinklerUpperBound", JaroWinklerUpperBound);
+  Options.store(Opts, "DiceUpperBound", DiceUpperBound);
+
+  Options.store(Opts, "LevenshteinLowerBound", LevenshteinLowerBound);
+  Options.store(Opts, "PrefixLowerBound", PrefixLowerBound);
+  Options.store(Opts, "SuffixLowerBound", SuffixLowerBound);
+  Options.store(Opts, "SubstringLowerBound", SubstringLowerBound);
+  Options.store(Opts, "JaroWinklerLowerBound", JaroWinklerLowerBound);
+  Options.store(Opts, "DiceLowerBound", DiceLowerBound);
+}
+
+// Matcher.
+void SuspiciousCallArgumentCheck::registerMatchers(MatchFinder *Finder) {
+  // Only match calls with at least 2 arguments.
+  Finder->addMatcher(
+      callExpr(unless(argumentCountIs(0)), unless(argumentCountIs(1)))
+          .bind("functionCall"),
+      this);
+}
+
+void SuspiciousCallArgumentCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *MatchedCallExpr =
+      Result.Nodes.getNodeAs<CallExpr>("functionCall");
+
+  const Decl *CalleeDecl = MatchedCallExpr->getCalleeDecl();
+  if (!CalleeDecl)
+    return;
+
+  const FunctionDecl *CalleeFuncDecl = CalleeDecl->getAsFunction();
+  if (!CalleeFuncDecl)
+    return;
+
+  // Get param attributes.
+  setParamNamesAndTypes(CalleeFuncDecl);
+
+  if (ParamNames.empty())
+    return;
+
+  // Get Arg attributes.
+  // Lambda functions first Args are themselves.
+  unsigned InitialArgIndex = 0;
+
+  if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(CalleeFuncDecl)) {
+    if (MethodDecl->getParent()->isLambda())
+      InitialArgIndex = 1;
+  }
+
+  setArgNamesAndTypes(MatchedCallExpr, InitialArgIndex);
+
+  if (ArgNames.empty())
+    return;
+
+  // In case of variadic functions.
+  unsigned ParamCount = ParamNames.size();
+
+  // Check similarity.
+  for (unsigned i = 0; i < ParamCount; i++) {
+    for (unsigned j = i + 1; j < ParamCount; j++) {
+      // Do not check if param or arg names are short, or not convertible.
+      if (!areParamAndArgComparable(i, j, Result.Context))
+        continue;
+
+      if (areArgsSwapped(i, j)) {
+        // Warning at the function call.
+        diag(MatchedCallExpr->getBeginLoc(), "%0 (%1) is swapped with %2 (%3).")
+            << ArgNames[i] << ParamNames[i] << ArgNames[j] << ParamNames[j];
+
+        // Note at the functions declaration.
+        diag(CalleeFuncDecl->getBeginLoc(),
+             "%0 is declared here:", DiagnosticIDs::Note)
+            << CalleeFuncDecl->getNameInfo().getName().getAsString();
+
+        return; // TODO: Address this return later
+      }
+    }
+  }
+}
+
+void SuspiciousCallArgumentCheck::setParamNamesAndTypes(
+    const FunctionDecl *CalleeFuncDecl) {
+  // Reset vectors, and fill them with the currently checked function's
+  // attributes.
+  ParamNames.clear();
+  ParamTypes.clear();
+
+  for (unsigned i = 0, e = CalleeFuncDecl->getNumParams(); i != e; ++i) {
+
+    if (const ParmVarDecl *PVD = CalleeFuncDecl->getParamDecl(i)) {
+      ParamTypes.push_back(PVD->getType());
+
+      if (IdentifierInfo *II = PVD->getIdentifier()) {
+        ParamNames.push_back(II->getName());
+        continue;
+      } else {
+        ParamNames.push_back(StringRef());
+        continue;
+      }
+    }
+    ParamTypes.push_back(QualType());
+    ParamNames.push_back(StringRef());
+  }
+}
+
+void SuspiciousCallArgumentCheck::setArgNamesAndTypes(
+    const CallExpr *MatchedCallExpr, unsigned InitialArgIndex) {
+  // Reset vectors, and fill them with the currently checked function's
+  // attributes.
+  ArgNames.clear();
+  ArgTypes.clear();
+
+  for (unsigned i = InitialArgIndex, j = MatchedCallExpr->getNumArgs(); i < j;
+       i++) {
+    if (const auto *DeclRef = dyn_cast<DeclRefExpr>(
+            MatchedCallExpr->getArg(i)->IgnoreParenImpCasts())) {
+      if (const auto *Var = dyn_cast<VarDecl>(DeclRef->getDecl())) {
+
+        ArgTypes.push_back(Var->getType());
+        ArgNames.push_back(Var->getName());
+        continue;
+      } else if (const auto *Var = dyn_cast<FunctionDecl>(DeclRef->getDecl())) {
+        ArgTypes.push_back(Var->getType());
+        ArgNames.push_back(Var->getName());
+        continue;
+      }
+    }
+    ArgTypes.push_back(QualType());
+    ArgNames.push_back(StringRef());
+  }
+}
+
+bool SuspiciousCallArgumentCheck::areParamAndArgComparable(
+    unsigned Position1, unsigned Position2, const ASTContext *Ctx) const {
+  if (Position1 >= ArgNames.size() || Position2 >= ArgNames.size())
+    return false;
+  if (ArgNames[Position1].size() < 3 || ArgNames[Position2].size() < 3 ||
+      ParamNames[Position1].size() < 3 || ParamNames[Position2].size() < 3)
+    return false;
+  if (!areTypesCompatible(ArgTypes[Position1], ParamTypes[Position2], Ctx) ||
+      !areTypesCompatible(ArgTypes[Position2], ParamTypes[Position1], Ctx))
+    return false;
+  return true;
+}
+
+bool SuspiciousCallArgumentCheck::areArgsSwapped(unsigned Position1,
+                                                 unsigned Position2) const {
+  bool param1Arg2NamesSimilar =
+      areNamesSimilar(ArgNames[Position2], ParamNames[Position1], Bound::Upper);
+  bool param2Arg1NamesSimilar =
+      areNamesSimilar(ArgNames[Position1], ParamNames[Position2], Bound::Upper);
+  bool param1Arg1NamesSimilar =
+      areNamesSimilar(ArgNames[Position1], ParamNames[Position1], Bound::Lower);
+  bool param2Arg2NamesSimilar =
+      areNamesSimilar(ArgNames[Position2], ParamNames[Position2], Bound::Lower);
+
+  return (param1Arg2NamesSimilar || param2Arg1NamesSimilar) &&
+         !param1Arg1NamesSimilar && !param2Arg2NamesSimilar;
+}
+
+bool SuspiciousCallArgumentCheck::areNamesSimilar(StringRef Arg,
+                                                  StringRef Param,
+                                                  Bound bound) const {
+
+  bool areNamesSimilar = false;
+  for (Heuristic Heur : AppliedHeuristics)
+    switch (Heur) {
+    case Heuristic::Equality:
+      areNamesSimilar |= applyEqualityHeuristic(Arg, Param);
+      break;
+    case Heuristic::Abbreviation:
+      areNamesSimilar |= applyAbbreviationHeuristic(Arg, Param);
+      break;
+    case Heuristic::Levenshtein:
+      areNamesSimilar |= applyLevenshteinHeuristic(Arg, Param,
+                                                   bound == Bound::Upper
+                                                       ? LevenshteinUpperBound
+                                                       : LevenshteinLowerBound);
+      break;
+    case Heuristic::Prefix:
+      areNamesSimilar |= applyPrefixHeuristic(
+          Arg, Param,
+          bound == Bound::Upper ? PrefixUpperBound : PrefixLowerBound);
+      break;
+    case Heuristic::Suffix:
+      areNamesSimilar |= applySuffixHeuristic(
+          Arg, Param,
+          bound == Bound::Upper ? SuffixUpperBound : SuffixLowerBound);
+      break;
+    case Heuristic::Substring:
+      areNamesSimilar |= applySubstringHeuristic(
+          Arg, Param,
+          bound == Bound::Upper ? SubstringUpperBound : SubstringLowerBound);
+      break;
+    case Heuristic::JaroWinkler:
+      areNamesSimilar |= applyJaroWinklerHeuristic(Arg, Param,
+                                                   bound == Bound::Upper
+                                                       ? JaroWinklerUpperBound
+                                                       : JaroWinklerLowerBound);
+      break;
+    case Heuristic::Dice:
+      areNamesSimilar |= applyDiceHeuristic(
+          Arg, Param, bound == Bound::Upper ? DiceUpperBound : DiceLowerBound);
+      break;
+    default:
+      break;
+    }
+  return areNamesSimilar;
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`misc-no-recursion
   Finds recursive functions and diagnoses them.
 
 - New :doc:`objc-nsinvocation-argument-lifetime
   <clang-tidy/checks/objc-nsinvocation-argument-lifetime>` check.
 
   Finds calls to ``NSInvocation`` methods under ARC that don't have proper
   argument object lifetimes.
 
+- New `readability-suspicious-call-argument
+  <clang-tidy/checks/readability-suspicious-call-argument>`_ check
+
+  This checker finds those function calls where the function arguments are
+  provided in an incorrect order.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
 - New alias :doc:`cert-con36-c
   <clang-tidy/checks/cert-con36-c>` to
   :doc:`bugprone-spuriously-wake-up-functions
   <clang-tidy/checks/bugprone-spuriously-wake-up-functions>` was added.
 

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `readability-redundant-smartptr-get <readability-redundant-smartptr-get.html>`_, "Yes"
    `readability-redundant-string-cstr <readability-redundant-string-cstr.html>`_,
    `readability-redundant-string-init <readability-redundant-string-init.html>`_, "Yes"
    `readability-simplify-boolean-expr <readability-simplify-boolean-expr.html>`_, "Yes"
    `readability-simplify-subscript-expr <readability-simplify-subscript-expr.html>`_, "Yes"
    `readability-static-accessed-through-instance <readability-static-accessed-through-instance.html>`_, "Yes"
    `readability-static-definition-in-anonymous-namespace <readability-static-definition-in-anonymous-namespace.html>`_, "Yes"
    `readability-string-compare <readability-string-compare.html>`_, "Yes"
+   `readability-suspicious-call-argument <readability-suspicious-call-argument.html>`_, "Yes"
    `readability-uniqueptr-delete-release <readability-uniqueptr-delete-release.html>`_, "Yes"
    `readability-uppercase-literal-suffix <readability-uppercase-literal-suffix.html>`_, "Yes"
    `zircon-temporary-objects <zircon-temporary-objects.html>`_,
 
 
 .. csv-table:: Aliases..
    :header: "Name", "Redirect", "Offers fixes"
 

clang-tools-extra/docs/clang-tidy/checks/readability-suspicious-call-argument.rst

+.. title:: clang-tidy - readability-suspicious-call-argument
+
+readability-suspicious-call-argument
+====================================
+
+This checker finds those function calls where the function arguments are
+provided in an incorrect order. It compares the name of the given variable
+to the argument name in the function definition.
+
+It issues a message if the given variable name is similar to an another
+function argument in a function call. It uses case insensitive comparison.

clang-tools-extra/test/clang-tidy/checkers/readability-suspicious-call-argument.cpp

+// RUN: %check_clang_tidy %s readability-suspicious-call-argument %t -- -- -std=c++11
+
+void foo_1(int aaaaaa, int bbbbbb) {}
+
+void foo_2(int source, int aaaaaa) {}
+
+void foo_3(int valToRet, int aaaaaa) {}
+
+void foo_4(int pointer, int aaaaaa) {}
+
+void foo_5(int aaaaaa, int bbbbbb, int cccccc, ...) {}
+
+void foo_6(const int dddddd, bool &eeeeee) {}
+
+void foo_7(int aaaaaa, int bbbbbb, int cccccc, int ffffff = 7) {}
+
+// Test functions for convertible argument--parameter types.
+void fun(const int &m);
+void fun2() {
+  int m = 3;
+  fun(m);
+}
+
+// Test cases for parameters of const reference and value.
+void value_const_reference(int llllll, const int &kkkkkk);
+
+void const_ref_value_swapped() {
+  const int &kkkkkk = 42;
+  const int &llllll = 42;
+  value_const_reference(kkkkkk, llllll);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: kkkkkk (llllll) is swapped with llllll (kkkkkk). [readability-suspicious-call-argument]
+}
+
+// Const, non const references.
+void const_nonconst_parameters(const int &mmmmmm, int &nnnnnn);
+
+void const_nonconst_swap1() {
+  const int &nnnnnn = 42;
+  int mmmmmm;
+  // Do not check, because non-const reference parameter cannot bind to const reference argument.
+  const_nonconst_parameters(nnnnnn, mmmmmm);
+}
+
+void const_nonconst_swap3() {
+  const int nnnnnn = 42;
+  int m = 42;
+  int &mmmmmm = m;
+  // Do not check, const int does not bind to non const reference.
+  const_nonconst_parameters(nnnnnn, mmmmmm);
+}
+
+void const_nonconst_swap2() {
+  int nnnnnn;
+  int mmmmmm;
+  // Check for swapped arguments. (Both arguments are non-const.)
+  const_nonconst_parameters(
+      nnnnnn,
+      mmmmmm);
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: nnnnnn (mmmmmm) is swapped with mmmmmm (nnnnnn). [readability-suspicious-call-argument]
+}
+
+void const_nonconst_pointers(const int *mmmmmm, int *nnnnnn);
+void const_nonconst_pointers2(const int *mmmmmm, const int *nnnnnn);
+
+void const_nonconst_pointers_swapped() {
+  int *mmmmmm;
+  const int *nnnnnn;
+  const_nonconst_pointers(nnnnnn, mmmmmm);
+}
+
+void const_nonconst_pointers_swapped2() {
+  const int *mmmmmm;
+  int *nnnnnn;
+  const_nonconst_pointers2(nnnnnn, mmmmmm);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: nnnnnn (mmmmmm) is swapped with mmmmmm (nnnnnn). [readability-suspicious-call-argument]
+}
+
+// Test cases for pointers and arrays.
+void pointer_array_parameters(
+    int *pppppp, int qqqqqq[4]);
+
+void pointer_array_swap() {
+  int qqqqqq[5];
+  int *pppppp;
+  // Check for swapped arguments. An array implicitly converts to a pointer.
+  pointer_array_parameters(qqqqqq, pppppp);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: qqqqqq (pppppp) is swapped with pppppp (qqqqqq). [readability-suspicious-call-argument]
+}
+
+// Test cases for multilevel pointers.
+void multilevel_pointer_parameters(int *const **pppppp,
+                                   const int *const *volatile const *qqqqqq);
+void multilevel_pointer_parameters2(
+    char *****nnnnnn, char *volatile *const *const *const *const &mmmmmm);
+
+typedef float T;
+typedef T *S;
+typedef S *const volatile R;
+typedef R *Q;
+typedef Q *P;
+typedef P *O;
+void multilevel_pointer_parameters3(float **const volatile ***rrrrrr, O &ssssss);
+
+void multilevel_pointer_swap() {
+  int *const **qqqqqq;
+  int *const **pppppp;
+  multilevel_pointer_parameters(qqqqqq, pppppp);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: qqqqqq (pppppp) is swapped with pppppp (qqqqqq). [readability-suspicious-call-argument]
+
+  char *****mmmmmm;
+  char *****nnnnnn;
+  multilevel_pointer_parameters2(mmmmmm, nnnnnn);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: mmmmmm (nnnnnn) is swapped with nnnnnn (mmmmmm). [readability-suspicious-call-argument]
+
+  float **const volatile ***rrrrrr;
+  float **const volatile ***ssssss;
+  multilevel_pointer_parameters3(ssssss, rrrrrr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ssssss (rrrrrr) is swapped with rrrrrr (ssssss). [readability-suspicious-call-argument]
+}
+
+void multilevel_pointer_parameters4(char ****pppppp,
+                                    char *const volatile **const *qqqqqq);
+void multilevel_pointer_parameters5(
+    bool *****nnnnnn, bool *volatile *const *const *const *&mmmmmm);
+void multilevel_pointer_parameters6(double **llllll, char **&kkkkkk);
+void multilevel_pointer_parameters7(const volatile int ***iiiiii,
+                                    const int *const *const *jjjjjj);
+
+void multilevel_pointer_swap3() {
+  char ****qqqqqq;
+  char *const volatile **const *pppppp;
+  // Do not check.
+  multilevel_pointer_parameters4(qqqqqq, pppppp);
+
+  bool *****mmmmmm;
+  bool *volatile *const *const *const *nnnnnn;
+  // Do not check.
+  multilevel_pointer_parameters5(mmmmmm, nnnnnn);
+
+  double **kkkkkk;
+  char **llllll;
+  multilevel_pointer_parameters6(kkkkkk, llllll);
+
+  const volatile int ***jjjjjj;
+  const int *const *const *iiiiii;
+  multilevel_pointer_parameters7(jjjjjj, iiiiii);
+}
+
+// Test cases for multidimesional arrays.
+void multilevel_array_parameters(int pppppp[2][2][2], const int qqqqqq[][2][2]);
+
+void multilevel_array_parameters2(int (*mmmmmm)[2][2], int nnnnnn[9][2][23]);
+
+void multilevel_array_parameters3(int (*eeeeee)[2][2], int (&ffffff)[1][2][2]);
+
+void multilevel_array_parameters4(int (*llllll)[2][2], int kkkkkk[2][2]);
+
+void multilevel_array_parameters5(int iiiiii[2][2], char jjjjjj[2][2]);
+
+void multilevel_array_parameters6(int (*bbbbbb)[2][2], int cccccc[1][2][2]);
+
+void multilevel_array_swap() {
+  int qqqqqq[1][2][2];
+  int pppppp[][2][2] = {{{1, 2}, {1, 2}}, {{1, 2}, {1, 2}}}; // int [2][2][2]
+  multilevel_array_parameters(qqqqqq, pppppp);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: qqqqqq (pppppp) is swapped with pppppp (qqqqqq). [readability-suspicious-call-argument]
+
+  int(*nnnnnn)[2][2];
+  int mmmmmm[9][2][23];
+  // Do not check, array sizes has to match in every dimension, except the first.
+  multilevel_array_parameters2(nnnnnn, mmmmmm);
+
+  int ffffff[][2][2] = {{{1, 2}, {1, 2}}, {{1, 2}, {1, 2}}}; // int [2][2][2]
+  int eeeeee[1][2][2] = {{{1, 2}, {1, 2}}};                  // int [1][2][2]
+  // Do not check, for array references, size has to match in every dimension.
+  multilevel_array_parameters3(ffffff, eeeeee);
+
+  int kkkkkk[2][2][2];
+  int(*llllll)[2];
+  // Do not check, argument dimensions differ.
+  multilevel_array_parameters4(kkkkkk, llllll);
+
+  int jjjjjj[2][2];
+  char iiiiii[2][2];
+  // Do not check, array element types differ.
+  multilevel_array_parameters5(jjjjjj, iiiiii);
+
+  int t[][2][2] = {{{1, 2}, {1, 2}}, {{1, 2}, {1, 2}}}; // int [2][2][2]
+  int(*cccccc)[2][2] = t;                               // int (*)[2][2]
+  int bbbbbb[][2][2] = {{{1, 2}, {1, 2}}};              // int [1][2][2]
+  multilevel_array_parameters6(cccccc, bbbbbb);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: cccccc (bbbbbb) is swapped with bbbbbb (cccccc). [readability-suspicious-call-argument]
+}
+
+void multilevel_array_swap2() {
+  int qqqqqq[2][2][2];
+  const int pppppp[][2][2] = {{{1, 2}, {1, 2}}, {{1, 2}, {1, 2}}};
+  // Do not check, pppppp is const and cannot bind to an array with nonconst elements.
+  multilevel_array_parameters(qqqqqq, pppppp);
+}
+
+// Complex test case.
+void multilevel_pointer_array_parameters(const int(*const (*volatile const (*const (*const (*const &aaaaaa)[1])[32])[4])[3][2][2]), const int(*const (*volatile const (*const (*const (*&bbbbbb)[1])[32])[4])[3][2][2]));
+
+void multilevel_pointer_array_swap() {
+  const int(
+          *const(*volatile const(*const(*const(*aaaaaa)[1])[32])[4])[3][2][2]);
+  const int(
+          *const(*volatile const(*const(*const(*bbbbbb)[1])[32])[4])[3][2][2]);
+  multilevel_pointer_array_parameters(bbbbbb, aaaaaa);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bbbbbb (aaaaaa) is swapped with aaaaaa (bbbbbb). [readability-suspicious-call-argument]
+}
+
+enum class numbers_scoped { one,
+                            two };
+
+// Test cases for arithmetic types.
+void arithmetic_type_parameters(float vvvvvv, int wwwwww);
+void arithmetic_type_parameters2(numbers_scoped vvvvvv, int wwwwww);
+
+void arithmetic_types_swap1() {
+  bool wwwwww;
+  float vvvvvv;
+  arithmetic_type_parameters(wwwwww, vvvvvv);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: wwwwww (vvvvvv) is swapped with vvvvvv (wwwwww). [readability-suspicious-call-argument]
+}
+
+void arithmetic_types_swap3() {
+  char wwwwww;
+  unsigned long long int vvvvvv;
+  arithmetic_type_parameters(wwwwww, vvvvvv);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: wwwwww (vvvvvv) is swapped with vvvvvv (wwwwww). [readability-suspicious-call-argument]
+}
+
+void arithmetic_types_swap4() {
+  enum numbers { one,
+                 two };
+  numbers wwwwww = numbers::one;
+  int vvvvvv;
+  arithmetic_type_parameters(wwwwww, vvvvvv);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: wwwwww (vvvvvv) is swapped with vvvvvv (wwwwww). [readability-suspicious-call-argument]
+}
+
+void arithmetic_types_swap5() {
+  wchar_t vvvvvv;
+  float wwwwww;
+  arithmetic_type_parameters(wwwwww, vvvvvv);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: wwwwww (vvvvvv) is swapped with vvvvvv (wwwwww). [readability-suspicious-call-argument]
+}
+
+void arithmetic_types_swap6() {
+  wchar_t vvvvvv;
+  numbers_scoped wwwwww = numbers_scoped::one;
+  // Do not check, numers is a scoped enum type.
+  arithmetic_type_parameters2(wwwwww, vvvvvv);
+}
+
+// Base, derived
+class TestClass {
+public:
+  void thisFunction(int integerParam, int thisIsPARAM) {}
+};
+
+class DerivedTestClass : public TestClass {};
+
+void base_derived_pointer_parameters(TestClass *aaaaaa,
+                                     DerivedTestClass *bbbbbb);
+
+void base_derived_swap1() {
+  TestClass *bbbbbb;
+  DerivedTestClass *aaaaaa;
+  // Do not check, because TestClass does not convert implicitly to DerivedTestClass.
+  base_derived_pointer_parameters(bbbbbb, aaaaaa);
+}
+
+void base_derived_swap2() {
+  DerivedTestClass *bbbbbb, *aaaaaa;
+  // Check for swapped arguments, DerivedTestClass converts to TestClass implicitly.
+  base_derived_pointer_parameters(bbbbbb, aaaaaa);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bbbbbb (aaaaaa) is swapped with aaaaaa (bbbbbb). [readability-suspicious-call-argument]
+}
+
+// Multilevel inheritance
+class DerivedOfDerivedTestClass : public DerivedTestClass {};
+
+void multi_level_inheritance_swap() {
+  DerivedOfDerivedTestClass *aaaaaa, *bbbbbb;
+  // Check for swapped arguments. Derived classes implicitly convert to their base.
+  base_derived_pointer_parameters(
+      bbbbbb, aaaaaa);
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: bbbbbb (aaaaaa) is swapped with aaaaaa (bbbbbb). [readability-suspicious-call-argument]
+}
+
+// Tests for function pointer swaps
+void funct_ptr_params(double (*ffffff)(int, int), double (*gggggg)(int, int));
+void funct_ptr_params(double (*ffffff)(int, int), int (*gggggg)(int, int));
+
+double ffffff(int a, int b) { return 0; }
+double gggggg(int a, int b) { return 0; }
+
+void funtionc_ptr_params_swap() {
+  funct_ptr_params(gggggg, ffffff);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: gggggg (ffffff) is swapped with ffffff (gggggg). [readability-suspicious-call-argument]
+}
+
+int fffff(int a, int b) { return 0; }
+
+void function_ptr_swap2() {
+  // Do not check, because the function `ffffff` cannot convert to a function
+  // with prototype: double(int,int).
+  funct_ptr_params(gggggg, fffff);
+}
+
+int main() {
+
+  // Equality test.
+  int aaaaaa, cccccc = 0;
+  foo_1(cccccc, aaaaaa); // Should fail.
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: cccccc (aaaaaa) is swapped with aaaaaa (bbbbbb). [readability-suspicious-call-argument]
+
+  // Abbreviation test.
+  int src = 0;
+  foo_2(aaaaaa, src); // Should fail.
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: aaaaaa (source) is swapped with src (aaaaaa). [readability-suspicious-call-argument]
+
+  // Levenshtein test.
+  int aaaabb = 0;
+  foo_1(cccccc, aaaabb); // Should fail.
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: cccccc (aaaaaa) is swapped with aaaabb (bbbbbb). [readability-suspicious-call-argument]
+
+  // Prefix test.
+  int aaaa = 0;
+  foo_1(cccccc, aaaa); // Should fail.
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: cccccc (aaaaaa) is swapped with aaaa (bbbbbb). [readability-suspicious-call-argument]
+
+  // Suffix test.
+  int urce = 0;
+  foo_2(cccccc, urce); // Should fail.
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: cccccc (source) is swapped with urce (aaaaaa). [readability-suspicious-call-argument]
+
+  // Substring test.
+  int ourc = 0;
+  foo_2(cccccc, ourc); // Should fail.
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: cccccc (source) is swapped with ourc (aaaaaa). [readability-suspicious-call-argument]
+
+  // Jaro-Winkler test.
+  int iPonter = 0;
+  foo_4(cccccc, iPonter); // Should fail.
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: cccccc (pointer) is swapped with iPonter (aaaaaa). [readability-suspicious-call-argument]
+
+  // Dice test.
+  int aaabaa = 0;
+  foo_1(cccccc, aaabaa); // Should fail.
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: cccccc (aaaaaa) is swapped with aaabaa (bbbbbb). [readability-suspicious-call-argument]
+
+  // Variadic function test.
+  int bbbbbb = 0;
+  foo_5(src, bbbbbb, cccccc, aaaaaa); // Should pass.
+  foo_5(cccccc, bbbbbb, aaaaaa, src); // Should fail.
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: cccccc (aaaaaa) is swapped with aaaaaa (cccccc). [readability-suspicious-call-argument]
+
+  // Test function with default argument.
+  foo_7(src, bbbbbb, cccccc, aaaaaa); // Should fail.
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: src (aaaaaa) is swapped with aaaaaa (ffffff). [readability-suspicious-call-argument]
+
+  foo_7(cccccc, bbbbbb, aaaaaa, src); // Should fail.
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: cccccc (aaaaaa) is swapped with aaaaaa (cccccc). [readability-suspicious-call-argument]
+
+  int ffffff = 0;
+  foo_7(ffffff, bbbbbb, cccccc); // Should fail.
+
+  // Type match
+  bool dddddd = false;
+  int eeeeee = 0;
+  auto szam = 0;
+  foo_6(eeeeee, dddddd); // Should fail.
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: eeeeee (dddddd) is swapped with dddddd (eeeeee). [readability-suspicious-call-argument]
+  foo_1(szam, aaaaaa); // Should fail.
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: szam (aaaaaa) is swapped with aaaaaa (bbbbbb). [readability-suspicious-call-argument]
+
+  // Test lambda.
+  auto testMethod = [&](int method, int randomParam) { return 0; };
+  int method = 0;
+  testMethod(method, 0); // Should pass.
+
+  // Member function test
+  TestClass test;
+  int integ, thisIsAnArg = 0;
+  test.thisFunction(integ, thisIsAnArg); // Should pass.
+
+  return 0;
+}