Skip to content

Conversation

@geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Jul 1, 2022

More improvements to swift/string-length-conflation:

  • conflation in the String -> NSString direction is now detected 🎉
  • slightly more data flow is permitted (in particular through operators *, /)
  • additional test cases

Notably for the String and String.Index member functions, I can't find a way of checking the associated class is correct (i.e. that we're looking at a call to String.index not foo.index). While this would be nice to have, in practice since we match on the argument and parameter names as well, so I think false positives are unlikely.

@geoffw0 geoffw0 added no-change-note-required This PR does not need a change note Swift labels Jul 1, 2022
@geoffw0 geoffw0 requested a review from a team as a code owner July 1, 2022 14:08
Copy link
Contributor

@MathiasVP MathiasVP left a comment

Choose a reason for hiding this comment

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

LGTM! Indeed, what you write here:

Notably for the String and String.Index member functions, I can't find a way of checking the associated class is correct (i.e. that we're looking at a call to String.index not foo.index). While this would be nice to have, in practice since we match on the argument and parameter names as well, so I think false positives are unlikely.

is something we have to provide a better API for. Let's get this merged now so that it doesn't fall between the cracks as we're going on vacation.

@MathiasVP MathiasVP merged commit e98bdbf into github:main Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note Swift

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants