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

[apex] added fix to not tag single block for two error statements #3641

Closed
wants to merge 2 commits into from

Conversation

arunnc84
Copy link

@arunnc84 arunnc84 commented Nov 20, 2021

Added check to see if a violation of type EmptyIfStmt or EmptyWhileStmt or EmptyCatchBlock is already present not to add EmptyStatementBlock. Also handled the other scenario, if EmptyStatementBlock is already part of violation remove the violation and add the new violation of type EmptyIfStmt or EmptyWhileStmt or EmptyCatchBlock to the list.

Related issues

#3164

  • Fixes #

Ready?

Yes, wrote unit test and all unit tests passed also build and successfully tested in local.

Copy link
Member

@adangel adangel left a comment

Choose a reason for hiding this comment

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

Thanks for your pull request.
However, this is not the solution we want to have for the given problem.
Your solution now hides the root cause, that we have two rules that do the same thing. It furthermore makes the pmd-core module aware, that there some specific rules that need to be filtered out (we have now a cyclic dependency: pmd-apex depends on pmd-core, pmd-core depends on pmd-apex).

What we should do instead is the following (as outlined in the comment on #3164):

  1. Create a new rule combines all the Empty* rules that are already existing.
  2. Deprecate all the Empty* rules in favor of the new rule
  3. Use the new rule in quickstart.xml instead of the now deprecated rules
  4. In PMD7 delete all the deprecated Empty* rules

@adangel adangel changed the title added fix to not tag single block for two error statements [apex] added fix to not tag single block for two error statements Nov 23, 2021
@oowekyala
Copy link
Member

As explained by @adangel in #3641 (review), we cannot merge this fix. Refer to #3778 for details about the preferred fix.

@oowekyala oowekyala closed this Feb 12, 2022
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

3 participants