Skip to content

Issue #337: Replace three context strategy properties with a single contextStrategy property#421

Open
Brijeshthummar02 wants to merge 1 commit into
checkstyle:masterfrom
Brijeshthummar02:contextscope
Open

Issue #337: Replace three context strategy properties with a single contextStrategy property#421
Brijeshthummar02 wants to merge 1 commit into
checkstyle:masterfrom
Brijeshthummar02:contextscope

Conversation

@Brijeshthummar02
Copy link
Copy Markdown
Contributor

Issue #337

Problem

SuppressionJavaPatchFilter had three separate properties for controlling how the filter walks the AST when deciding whether to suppress a violation:

  • supportContextStrategyChecks — used the check's own node
  • checkNamesForContextStrategyByTokenOrParentSet — walked up to the parent node
  • checkNamesForContextStrategyByTokenOrAncestorSet — walked up to a specific ancestor node type

Dropping a check into the wrong property caused silent, incorrect suppression with zero indication anything went wrong. And the ancestor property had a hardcoded internal map that only covered a fixed set of built-in checks , custom checks were just left out with no workaround.

  • What i did Replaced all three with a single contextStrategy property:
<property name="contextStrategy"
    value="MethodLength:SELF, RightCurly:PARENT, FallThrough:LITERAL_SWITCH"/>

For more refer : #337 (comment)

The scope value accepts:

  • SELF / PARENT / ANCESTOR — direct replacements for the old three properties
  • Any Checkstyle TokenTypes constant (e.g. LITERAL_SWITCH, CLASS_DEF) — walks up to the first matching ancestor, which kills the need for the hardcoded map and finally makes ancestor-scope resolution available to custom checks

The old three properties are still around, marked @Deprecated(since="10.x", forRemoval=true), and behave exactly as before they just log a warning pointing to the new syntax. Nothing in existing configs breaks.

@Brijeshthummar02
Copy link
Copy Markdown
Contributor Author

@romani need your feedback and review.

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.

1 participant