rewrite-static-analysis icon indicating copy to clipboard operation
rewrite-static-analysis copied to clipboard

Add URLEqualHashCode Recipe

Open JLLeitschuh opened this issue 1 year ago • 3 comments

Based upon, but not completely fixing this: https://errorprone.info/bugpattern/URLEqualsHashCode

Signed-off-by: Jonathan Leitschuh [email protected]

What's changed?

Adds a recipe URLEqualsHashCode.

What's your motivation?

I was curious about trying out the new template recipe generation. This seemed like a fun way to try it out.

Anything in particular you'd like reviewers to focus on?

Are there any use cases/tests that I likely missed?

Anyone you would like to review specifically?

Have you considered any alternatives or workarounds?

Thought about writing the recipe not using the template engine, but this seemed like a good use case.

Also, the reason I'm using URI#toString to create a URI, instead of URL#toURI is because URL#toURI throws a checked exception

Any additional context

Checklist

  • [x] I've added unit tests to cover both positive and negative cases
  • [x] I've read and applied the recipe conventions and best practices
  • [x] I've used the IntelliJ IDEA auto-formatter on affected files

JLLeitschuh avatar Jan 28 '24 22:01 JLLeitschuh

Interesting. I suppose there are also legitimate uses (like file URLs). Otherwise any HashSet or HashMap would also be problematic. In this case the URLs are known to be for files: https://github.com/jetty/jetty.project/blob/5d9679adf861bc589fdb213758a88f16436f6286/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaInfConfiguration.java#L660

knutwannheden avatar Jan 28 '24 23:01 knutwannheden

Interesting insight. Not sure if there's a great way to detect that. In general, this could be an optionally applied recipe.

Dealing with collections could be super complicated, especially when the collection is returned/escapes the scope of the method.

JLLeitschuh avatar Jan 29 '24 01:01 JLLeitschuh

Is this good to merge?

JLLeitschuh avatar Feb 01 '24 14:02 JLLeitschuh