Skip to content

lint: Introduce scalafix Disable rules for partial (non-total) functions #137

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

Merged
merged 2 commits into from
Jan 31, 2020

Conversation

sideeffffect
Copy link
Contributor

I took inspiration from the Scalazzi project

@jakubjanecek
Copy link
Collaborator

In practice what does this do? I don't get the connection between catchNonFatal and URL encoding/decoding.

@sideeffffect
Copy link
Contributor Author

sideeffffect commented Jan 31, 2020

the static methods (encode/decode) on these classes, even though deterministic, are not total, and in some cases throw
that's why calling these should be wrapped in catchNonFatal

@jakubjanecek
Copy link
Collaborator

I don't get why only these two methods are mentioned. There must be many such methods so why just these two were chosen? We are not even using them in our code. Point is that either the list should be "full" or such rule does not make much sense - anyone working with Java APIs will definitely wrap them properly because you never know when exceptions can be thrown.

@sideeffffect
Copy link
Contributor Author

why only these two methods

because nobody put in the effort to add more. any help/PRs will be heartily appreciated! This is just a start -- my personal plan is to add more functions as time goes by, typically those that are widely used and throw or have any other suspicious behavior.

Copy link
Collaborator

@jakubjanecek jakubjanecek left a comment

Choose a reason for hiding this comment

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

I understand but in this current form the benefits of the rule are very limited and I like to keep as little code/configuration as possible. I would like to close this PR.

@jakubjanecek jakubjanecek merged commit dbec3e2 into avast:master Jan 31, 2020
@sideeffffect sideeffffect deleted the pimp-my-scalafix-4 branch January 31, 2020 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants