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

comparison with x.as_str() where x suffices #181

Open
llogiq opened this issue Aug 16, 2015 · 5 comments
Open

comparison with x.as_str() where x suffices #181

llogiq opened this issue Aug 16, 2015 · 5 comments
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. S-needs-discussion Status: Needs further discussion before merging or work can be started T-middle Type: Probably requires verifiying types

Comments

@llogiq
Copy link
Contributor

llogiq commented Aug 16, 2015

We recently had some instances where we compared name.as_str() == s albeit there was an impl PartialEq<&str>.

I say this should either be allowed by default or use a blacklist of types where we know that the aforementioned impl has the same semantics as the conversion + str-equality.

@llogiq llogiq added E-medium Call for participation: Medium difficulty level problem and requires some initial experience. T-middle Type: Probably requires verifiying types A-lint Area: New lints S-needs-discussion Status: Needs further discussion before merging or work can be started labels Aug 16, 2015
@Manishearth
Copy link
Member

+1 for a blacklist. If someone can come up with a sound way (no false positives) of doing it that's more precise, I'd love to hear it (matching on an as_str is as precise as the blacklist)

@llogiq
Copy link
Contributor Author

llogiq commented Aug 17, 2015

The only possible sound way would be to prove that .as_str() is homomorphic regarding equality (∀x:X where x.as_str() -> &str.∀s:&str.x.as_str() == sx == s).

While this may be easily solveable for a small subset (where PartialEq<&str> is implemented by calling as_str() first, then comparing), it is impossible to solve in the general case, and probably quite unwieldy to solve automatically in most useful cases.

@llogiq
Copy link
Contributor Author

llogiq commented Aug 17, 2015

We should probably blacklist (type, method_name) tuples.

@Manishearth
Copy link
Member

sgtm

@llogiq
Copy link
Contributor Author

llogiq commented Aug 18, 2015

Thinking a bit more about this, a generic (type, method_name, message) tuple + some code to match could subsume a good number of lints we currently have. We'd just need a way to optionally insert snippets into the message...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. S-needs-discussion Status: Needs further discussion before merging or work can be started T-middle Type: Probably requires verifiying types
Projects
None yet
Development

No branches or pull requests

2 participants