Skip to content

Conversation

mohamedsamehsalah
Copy link
Contributor

@mohamedsamehsalah mohamedsamehsalah commented Aug 4, 2024

  • Perhaps opinionated, but personally, I find the Class.class:cast alternative more readable, especially when working with Streams.
  • One downside to this rule, is that we lose the compile-time cast checks in exchange for runtime checks.

What are your guys thoughts?

Suggested commit message

Introduce `Class(Reference|Literal)Cast` class rules

Copy link

github-actions bot commented Aug 4, 2024

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

return s -> clazz.isInstance(s);
}

Function<Number, Integer> testClassLiteralCast() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Curious why the pevious three tests threw exceptions as part of their signature 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Looks like a copy-paste error; will drop!

@mohamedsamehsalah mohamedsamehsalah added this to the 0.18.0 milestone Aug 4, 2024
Copy link
Member

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

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

I don't think that this impacts type safety, and it matches the Error Prone Support code style.

Added a commit. Suggested commit message:

Introduce `Class{Literal,Reference}Cast` Refaster rules (#1269)

}
}

/** Prefer {@link Class#cast(Object)} method references over more verbose alternatives. */
Copy link
Member

Choose a reason for hiding this comment

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

"Verbose" is perhaps a bit strong/unclear. I see that this mirrors the rules above, though. Will propose something.

return s -> clazz.isInstance(s);
}

Function<Number, Integer> testClassLiteralCast() {
Copy link
Member

Choose a reason for hiding this comment

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

Looks like a copy-paste error; will drop!

Comment on lines 33 to 34
Class<? extends Integer> clazz = Integer.class;
return i -> clazz.cast(i);
Copy link
Member

Choose a reason for hiding this comment

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

We don't need the separate variable here. (It's used above to test for a non-matching scenario.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah! Makes sense 👍

@Stephan202 Stephan202 changed the title Introduce Class(Reference|Literal)Cast class rules Introduce Class{Literal,Reference}Cast Refaster rules Aug 4, 2024
Copy link

github-actions bot commented Aug 4, 2024

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@rickie rickie force-pushed the mohamedsamehsalah/clas-literal-cast branch from 543cf41 to a073a27 Compare August 10, 2024 14:50
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202 Stephan202 modified the milestones: 0.18.0, 0.19.0 Aug 11, 2024
@rickie rickie modified the milestones: 0.19.0, 0.18.0 Aug 11, 2024
@rickie rickie force-pushed the mohamedsamehsalah/clas-literal-cast branch from a073a27 to 553df03 Compare August 11, 2024 12:34
Copy link

Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202 Stephan202 merged commit 3d9aab7 into master Aug 11, 2024
@Stephan202 Stephan202 deleted the mohamedsamehsalah/clas-literal-cast branch August 11, 2024 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants