Issue #13809: Kill mutation in XpathUtil

[Kevin222004]

Issue #13809: Kill mutation in XpathUtil

checkstyle/config/pitest-suppressions/pitest-utils-suppressions.xml

Lines 354 to 361 in 0322ba1

	<mutation unstable="false">
	<sourceFile>XpathUtil.java</sourceFile>
	<mutatedClass>com.puppycrawl.tools.checkstyle.utils.XpathUtil</mutatedClass>
	<mutatedMethod>getXpathItems</mutatedMethod>
	<mutator>org.pitest.mutationtest.engine.gregor.mutators.experimental.NakedReceiverMutator</mutator>
	<description>replaced call to java/util/stream/Stream::map with receiver</description>
	<lineContent>.map(NodeInfo.class::cast)</lineContent>
	</mutation>

[nrmancuso]

We need to understand how pitest is actually mutating this code before we make changes like this. The code proposed by this PR is objectively less clear and concise than what we have now. We should not allow pitest to decrease our code quality, especially if we don’t even understand what it is doing under the hood.

[romani]

items:

[romani]

Ok to merge.

@Vyom-Yadav , please share your opinion on cast problem, looks like this becomes hot topic.

[romani]

@Kevin222004 , please rebase to resolve conflict

[romani]

@Kevin222004 , please address Checker

[rdiachenko]

lgtm

@Kevin222004 please resolve checker errors

[Vyom-Yadav]

LGTM! Please fix CI errors.

[nrmancuso]

Item:

[nrmancuso]

Why did we make this about collections, when the issue is with the calls to Class#cast? If we want to make some hack to bypass pitest, why not just have a util method to cast the stream as an intermediate operation?

Pseudocode:

<R, T> Stream<T> castedStreamOf(Stream<R> s, Class<T> c) {
  return stream.map(c::cast)
}

Then we can collect or perform subsequent stream operations elsewhere more readily.

[romani]

It has unmodified collection, that pitest will complain about, not sure why it is not doing it now.

And current implementation is easier, and we don't want to make it highly reusable, at least for now.

[nrmancuso]

Why is it a public method in a utility class if we don't want it to be reusable?

[romani]

Pitest limitation to allow easy exclude from mutation by class name .

Whole code of this class is excluded from mutation at all. And class of exist until pitest improve. We did not report issue yet, but we collecting unfortunate cases in our issue

Outdated

[nrmancuso]

The changes here don't add up for me. In order to remove one pitest suppression (that should just be permanent IMO), we have added:

• 2 checker suppressions (tech debt)
• avoidCallsTo suppression in pom.xml (tech debt)
• yet another specialized utility method that won't get used anywhere else (tech debt)
• Production code (the reason why we are really here) that is much less clear than before

At this point, we are shuffling tech debt around. I won't block this, but I cannot approve either.

[nrmancuso]

@romani let’s just merge this and move on. We have already spent too much time and resources on this PR.