[java] Renderers output wrong class qualified name for nested classes

[cowwoc]

Affects PMD Version: 7.0.0

Rule: AbstractClassWithoutAbstractMethod

Description: The AbstractClassWithoutAbstractMethod rule is being triggered on a non-abstract class

Code Sample demonstrating the issue:

/*
 * Copyright (c) 2024 Gili Tzabari
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 * http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
package com.github.cowwoc.lumina;

import com.fasterxml.jackson.databind.JsonNode;

import java.net.URI;
import java.time.Instant;
import java.time.format.DateTimeParseException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.UUID;

import static com.github.cowwoc.requirements.java.DefaultJavaValidators.requireThat;

/**
 * Helper methods for manipulating object properties.
 */
public final class Property
{
	private final String name;
	private final JsonNode value;
	private final boolean insideStateMetadata;

	/**
	 * Wraps a JsonNode.
	 *
	 * @param name                the name of the property
	 * @param value               the value of the property
	 * @param insideStateMetadata true if the property is declared inside a {@code @state} metadata property
	 * @throws NullPointerException if any of the arguments are null
	 */
	public Property(String name, JsonNode value, boolean insideStateMetadata)
	{
		requireThat(name, "name").isNotNull();
		requireThat(value, "value").isNotNull();
		this.name = name;
		this.value = value;
		this.insideStateMetadata = insideStateMetadata;
	}

	/**
	 * @return true if the property name belongs to a metadata property
	 */
	public boolean isMetadata()
	{
		return !insideStateMetadata && name.startsWith("@");
	}

	/**
	 * @return true if the property name belongs to a state property
	 */
	public boolean isState()
	{
		return !isMetadata();
	}

	/**
	 * @return the String value
	 * @throws IllegalArgumentException if the value is not a string
	 */
	public String stringValue()
	{
		if (!value.isTextual())
		{
			throw new IllegalArgumentException(name + " must be a string.\n" +
				"Actual  : " + value.getNodeType() + "\n" +
				"Node    : " + value + "\n" +
				"Resource: " + this);
		}
		return value.textValue();
	}

	/**
	 * @return the String value as a URI
	 * @throws IllegalArgumentException if the value is not a valid URI
	 */
	public URI uriValue()
	{
		String string = stringValue();
		return requireThat(string, name).asUri().getValue();
	}

	/**
	 * @return the value as an array of strings
	 * @throws IllegalArgumentException if the value is not an array of strings
	 */
	public List<String> stringValues()
	{
		if (!value.isArray())
		{
			throw new IllegalArgumentException(name + " must be an array.\n" +
				"Actual  : " + value.getNodeType() + "\n" +
				"Node    : " + value + "\n" +
				"Resource: " + this);
		}

		List<String> elements = new ArrayList<>();
		for (JsonNode element : value)
			elements.add(new Property(name, element, insideStateMetadata).stringValue());
		return elements;
	}

	/**
	 * @return the UUID value
	 * @throws IllegalArgumentException if the value is not a UUID
	 */
	public UUID uuidValue()
	{
		return UUID.fromString(stringValue());
	}

	/**
	 * @return the Instant value
	 * @throws DateTimeParseException if the value is not an Instant
	 */
	public Instant instantValue()
	{
		return Instant.parse(stringValue());
	}

	/**
	 * @return the {@code Map<Integer, String>} value
	 * @throws IllegalArgumentException if the value is not a {@code Map<Integer, String>}
	 */
	public Map<Integer, String> integerToStringValue()
	{
		Map<Integer, String> map = new HashMap<>();
		for (Entry<String, JsonNode> entry : value.properties())
		{
			Integer version = requireThat(entry.getKey(), "key").asInteger().getValue();
			String nestedValue = new Property(entry.getKey(), entry.getValue(), insideStateMetadata).stringValue();
			map.put(version, nestedValue);
		}
		return map;
	}
}

Expected outcome: No AbstractClassWithoutAbstractMethod violation
Actual outcome: [INFO] PMD Failure: com.github.cowwoc.lumina.Property:38 Rule:AbstractClassWithoutAbstractMethod Priority:3 This abstract class does not have any abstract methods.

Running PMD through: Maven with the following configuration:

<properties>
		<pmd.version>7.0.0</pmd.version>
</properties>

<build>
	<pluginManagement>
		<plugins>
			<plugin>
				<groupId>org.apache.maven.plugins</groupId>
				<artifactId>maven-pmd-plugin</artifactId>
				<version>3.21.2</version>
				<dependencies>
					<dependency>
						<!-- WORKAROUND: https://github.com/apache/maven-pmd-plugin/pull/144 -->
						<groupId>net.sourceforge.pmd</groupId>
						<artifactId>pmd-compat6</artifactId>
						<version>${pmd.version}</version>
					</dependency>
					<dependency>
						<groupId>net.sourceforge.pmd</groupId>
						<artifactId>pmd-core</artifactId>
						<version>${pmd.version}</version>
					</dependency>
					<dependency>
						<groupId>net.sourceforge.pmd</groupId>
						<artifactId>pmd-java</artifactId>
						<version>${pmd.version}</version>
					</dependency>
					<dependency>
						<groupId>net.sourceforge.pmd</groupId>
						<artifactId>pmd-javascript</artifactId>
						<version>${pmd.version}</version>
					</dependency>
					<dependency>
						<groupId>net.sourceforge.pmd</groupId>
						<artifactId>pmd-jsp</artifactId>
						<version>${pmd.version}</version>
					</dependency>
				</dependencies>
			</plugin>
		</plugins>
	</pluginManagement>
</build>

[cowwoc]

PS: The rule is triggering on this line: private final JsonNode value;

[oowekyala]

The rule is actually triggered on the following place: https://github.com/cowwoc/lumina/blob/main/java/src/main/java/com/github/cowwoc/lumina/Form.java#L38

There is a problem with the renderer as it just takes the simple name of the class and prepends the package name. This of course is wrong when the type is a nested class like here. The name of the class is com.github.cowwoc.lumina.Form.Property and not com.github.cowwoc.lumina.Property. Since you have the latter in the package the renderer output is confusing.

I think the renderer should output file names and not class names, for instance:

src/main/java/com/github/cowwoc/lumina/Form.java:38 Rule:AbstractClassWithoutAbstractMethod Priority:3 This abstract class does not have any abstract methods.

instead of the current output now (broken)

com.github.cowwoc.lumina.Property:38 Rule:AbstractClassWithoutAbstractMethod Priority:3 This abstract class does not have any abstract methods.

or "fixed", but it is not obvious which file the line number is for:

com.github.cowwoc.lumina.Form.Property:38 Rule:AbstractClassWithoutAbstractMethod Priority:3 This abstract class does not have any abstract methods.

[oowekyala]

Here is the problem in the Maven renderer:

https://github.com/apache/maven-pmd-plugin/blob/c54170707cfd0aec4b5d9ee6083230f2551cdaa4/src/main/java/org/apache/maven/plugins/pmd/PmdViolationCheckMojo.java#L96-L104

But other renderers in PMD have the same problem, eg

pmd/pmd-core/src/main/java/net/sourceforge/pmd/renderers/TextColorRenderer.java

Lines 217 to 219 in e5f55c0

	String packageName = rv.getAdditionalInfo().getOrDefault(RuleViolation.PACKAGE_NAME, "");
	String className = rv.getAdditionalInfo().getOrDefault(RuleViolation.CLASS_NAME, "");
	return StringUtils.isNotBlank(packageName) ? packageName + '.' + className : "";

[jsotuyod]

@adangel I'm unsure this holds for the 3.22.0 plugin version, but you may want to take this to the maven plugin's JIRA.

[adangel]

For nested classes, what are the expected values in additionalinfo of rule violations?

pmd/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/internal/JavaViolationDecorator.java

Lines 41 to 42 in 6a328d6

	setIfNonNull(RuleViolation.CLASS_NAME, getClassName(javaNode), additionalInfo);
	setIfNonNull(RuleViolation.PACKAGE_NAME, javaNode.getRoot().getPackageName(), additionalInfo);

https://docs.pmd-code.org/latest/pmd_languages_java.html#violation-decorators

Do we miss something here? Like something like "FULLY_QUALIFIED_CLASS_NAME"?

[oowekyala]

Do we miss something here? Like something like "FULLY_QUALIFIED_CLASS_NAME"?

Yes. Maybe we can specify CLASS_NAME to mean eg here Form.Property for nested classes though.