Suppressions to XPath Suppressions

[rnveach]

Subset of #11604 ,

Clarification is needed here and in the code on what is the relationship between xpath suppressions and normal suppressions. There are a few conflicts in the code base and there is no simple answer on what this would mean if possibly another walker is added.

Resolving this can help clear up the code and possibly make the design simpler and more expandable in the future. Maybe we create a whole new interface to show the parent/child relationship, or maybe we split code apart to show no connection. It could also mean breaking the existing design for something that makes more sense.

For simplicity, lets say Checker is another form of Walker, it does no parsing but loads the files.

Are xpath suppressions a sub-filter of suppressions?

Currently suppressions and xpath suppressions code is in the same file, https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressionsLoader.java#L140 .

DTD also says the suppression file supports both xpath and non-xpath, https://github.com/checkstyle/checkstyle/blob/master/src/main/resources/com/puppycrawl/tools/checkstyle/suppressions_1_2_xpath_experimental.dtd#L8 .

This makes it seem like there is a parent-child relationship between the 2.

Are xpath suppressions a completely different type of suppressions and there is no similarity between them in hierarchy?

We store the filters in completely separate areas, https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressionsLoader.java#L116-L121 .

Even if we load them all from a single file, only the specific filter for each will activate each type of suppression. Meaning if we only add SuppressionXpathFilter to a config, it will not suppress anything from the non-xpath suppressions. It requires the SuppressionFilter to be added for those.

The DTD also shows that not everything from non-xpath is pulled into xpath. We dropped lines and columns in xpath, and xpath has the unique field query.

This makes it seem more like there is no real parent-child relationship besides them both being a form of suppression. I would more closely say that xpath is tree's filter, while non-xpath is checker's filter.

3rd option

Since there is always another option, is there a certain part that of suppressions that should be declared as parent/child and the other parts are actually customization of their respective walkers.

Or....

[nrmancuso]

From https://checkstyle.sourceforge.io/config_filters.html#SuppressionFilter:

A suppressions XML document contains a set of suppress elements, where each suppress element can have the following attributes:

• files - a Pattern matched against the file name associated with an audit event. It is optional.
• checks - a Pattern matched against the name of the check associated with an audit event. Optional as long as id or message is specified.
• message - a Pattern matched against the message of the check associated with an audit event. Optional as long as checks or id is specified.
• id - a String matched against the check id associated with an audit event. Optional as long as checks or message is specified.
• lines - a comma-separated list of values, where each value is an int or a range of integers denoted by integer-integer. It is optional.
• columns - a comma-separated list of values, where each value is an int or a range of integers denoted by integer-integer. It is optional.

Each audit event is checked against each suppress element. It is suppressed if all specified attributes match against the audit event.

---

From https://checkstyle.sourceforge.io/config_filters.html#SuppressionXpathFilter

A suppressions XML document contains a set of suppress and suppress-xpath elements, where each suppress-xpath element can have the following attributes:

• files - a Pattern matched against the file name associated with an audit event. It is optional.
• checks - a Pattern matched against the name of the check associated with an audit event. Optional as long as id or message is specified.
• message - a Pattern matched against the message of the check associated with an audit event. Optional as long as checks or id is specified.
• id - a String matched against the ID of the check associated with an audit event. Optional as long as checks or message is specified.
• query - a String xpath query. It is optional.

Each audit event is checked against each suppress and suppress-xpath element. It is suppressed if all specified attributes match against the audit event.

---

Idea: we create a Suppressible interface with setFiles, setChecks, setMessage, setId. Then the two implementations differ by setLines and setColumns (regular) and setQuery (xpath).

They differ by how to specify location of violation to suppress only.

---

Are xpath suppressions a sub-filter of suppressions?

No. They have an intersection, but one is not a subset of the other.

[rnveach]

Idea: we create a Suppressible interface with setFiles, setChecks, setMessage, setId. Then the two implementations differ by setLines and setColumns (regular) and setQuery (xpath).
They have an intersection, but one is not a subset of the other.

I like this.

Another topic if we proceed down this path, is that our current suppression file method doesn't allow expansion to the same suppressions file by 3rd parties. Unfortunately, the DTD says what we can and cannot added. For a 3rd party to make a new type of suppression based file, it has to be something unique. This is different from modules in the configuration file which allows 3rd parties to use the same structure. This require a drastic format change.