Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix edge case in InconsistentCapitalization #4801

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PBoddington
Copy link

@PBoddington PBoddington commented Feb 1, 2025

Currently, InconsistentCapitalization does not handle well the case that the class has multiple fields with names that are equivalent except for capitalization.

For example, the following code

class Test {
    
    private int abc;
    private int ABC;
    
    void foo(int ABC) {
    
    }
}

results in an error

[InconsistentCapitalization] Found the field 'abc' with the same name as the parameter 'ABC' but with different capitalization.

On the other hand, if the order of the fields is swapped, there is no match.

This proposed change improves the behaviour for this edge case, so that the order in which the fields are declared no longer matters. The new behaviour is that a parameter is a match if there is at least one field that is equivalent except for capitalization, but no field matches exactly.

@PBoddington PBoddington marked this pull request as ready for review February 1, 2025 18:31
Comment on lines +218 to +221
List<Symbol> matchedFields = fields.get(Ascii.toLowerCase(variableName));
if (!matchedFields.isEmpty()
&& matchedFields.stream().map(Symbol::toString).noneMatch(variableName::equals)) {
matchedParameters.put(getCurrentPath(), matchedFields.getFirst());
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chose to use a List because using matchedFields.getFirst() is the simplest way to choose a matching field in a manner that is obviously deterministic. It would have to be a very weird class for the performance of iterating over the list to be a concern. However, I'd happily change the ListMultimap to something like a Map<String, Map<String, Symbol>> if requested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant