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

Add RemovableMatchChecker #62

Closed
wants to merge 4 commits into from

Conversation

yzgw
Copy link

@yzgw yzgw commented Aug 8, 2013

Add checker to detect whether match statement is removable or not.

This checker finds removable "match" statements for collections.
Effective Scala suggests to use pattern matching directly in function definitions if possible.
For example, the checker accepts

list map {
  case Some(x) => x
  case None => default
}

but rejects

list map { item =>
  item match {
    case Some(x) => x
    case None => default
  }
}

This helps to write pattern matchings clearly.

@HairyFotr
Copy link
Contributor

👍

I have a suggestion for one more case:

list map { _ match {
  case Some(x) => x
  case None => default
}}

@yzgw
Copy link
Author

yzgw commented Aug 29, 2013

Sounds good. But I found some bug of the checker.
After fixing it, I want to try it.

@matthewfarwell
Copy link
Member

I don't think that this is the right approach for Scalastyle. For instance we can never guarantee that someone won't create a function called map or foreach, which this would then apply to. I think this sort of check would be better as a compiler plugin or similar.

@HairyFotr
Copy link
Contributor

I've now added this check to Linter (a static analysis compiler plugin): HairyFotr/linter@ae17ec4

@yzgw
Copy link
Author

yzgw commented Sep 13, 2013

Ok, I got it.
But I want a easy way to add own checker...

@HairyFotr
Great!
I have not known Linter. I will consider using it!

@matthewfarwell
Copy link
Member

I think that this check fits better in Linter than in Scalastyle - the solution for Scalastyle seems far too general. A different solution for Scalastyle would be to allow the use of external rules. See #25. Would this be an acceptable solution @yzgw?

@matthewfarwell
Copy link
Member

This check fits better into Linter than into Scalastyle. If you really wish to add this rule to Scalastyle, then you can add it as an external rule. See #25. This isn't implemented at the minute, but would be fairly easy to do.

kitbellew pushed a commit to kitbellew/scalastyle that referenced this pull request Jul 3, 2021
Co-authored-by: Michael Wizner <mwz@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants