Skip to content

Commit

Permalink
Fix custom PMD rules (#513)
Browse files Browse the repository at this point in the history
  • Loading branch information
@newtork
newtork authored Jul 29, 2024
1 parent ebadc87 commit 4b575ff
Show file tree
Hide file tree
Showing 16 changed files with 62 additions and 114 deletions.
1 change: 1 addition & 0 deletions ...tivity/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/AuthenticationType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ public enum AuthenticationType
@Getter
private final String identifier;

@Nonnull
@Override
public String toString()
{
Expand Down
5 changes: 2 additions & 3 deletions ...nectivity/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public enum DestinationType
this.identifier = identifier;
}

@Nonnull
@Override
public String toString()
{
Expand Down Expand Up @@ -100,9 +101,7 @@ public static DestinationType ofIdentifierOrDefault(
return ofIdentifier(identifier);
}
catch( final IllegalArgumentException e ) {
if( log.isWarnEnabled() ) {
log.warn("Identifier '{}' is not supported. Falling back to {}.", identifier, defaultDestinationType);
}
log.warn("Identifier '{}' is not supported. Falling back to {}.", identifier, defaultDestinationType);
return defaultDestinationType;
}
}
Expand Down
5 changes: 2 additions & 3 deletions ...rm-connectivity/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ProxyType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,7 @@ public enum ProxyType
return ofIdentifier(identifier);
}
catch( final IllegalArgumentException e ) {
if( log.isWarnEnabled() ) {
log.warn("Identifier '{}' is not supported. Falling back to {}.", identifier, defaultProxyType);
}
log.warn("Identifier '{}' is not supported. Falling back to {}.", identifier, defaultProxyType);
return defaultProxyType;
}
}
Expand Down Expand Up @@ -133,6 +131,7 @@ public static ProxyType ofIdentifierSensitive( @Nonnull final String identifier
throw new IllegalArgumentException("Unknown " + ProxyType.class.getSimpleName() + ": " + identifier + ".");
}

@Nonnull
@Override
public String toString()
{
Expand Down
2 changes: 2 additions & 0 deletions ...main/java/com/sap/cloud/sdk/cloudplatform/connectivity/SecurityConfigurationStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public enum SecurityConfigurationStrategy
*
* @return The default value
*/
@Nonnull
public static SecurityConfigurationStrategy getDefault()
{
return FROM_DESTINATION;
Expand Down Expand Up @@ -76,6 +77,7 @@ public static SecurityConfigurationStrategy ofIdentifierOrDefault( @Nullable fin
throw new IllegalArgumentException("No SecurityConfigurationStrategy found for identifier: " + identifier);
}

@Nonnull
@Override
public String toString()
{
Expand Down
1 change: 1 addition & 0 deletions ...client4/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClientWrapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ public HeaderElement[] getElements()
return new HeaderElement[0];
}

@Nonnull
@Override
public String toString()
{
Expand Down
1 change: 1 addition & 0 deletions ...ava/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceRetrievalStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public enum DestinationServiceRetrievalStrategy
this.identifier = identifier;
}

@Nonnull
@Override
public String toString()
{
Expand Down
1 change: 1 addition & 0 deletions ...com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTokenExchangeStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public enum DestinationServiceTokenExchangeStrategy
this.identifier = identifier;
}

@Nonnull
@Override
public String toString()
{
Expand Down
144 changes: 36 additions & 108 deletions codestyle/sdk_specific_pmd_rules.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,9 @@
<property name="xpath">
<value>
<![CDATA[
//ClassOrInterfaceBodyDeclaration
/FieldDeclaration[@Static]
//PrimaryExpression[
PrimaryPrefix/Name[@Image = "LoggerFactory.getLogger"] and
PrimarySuffix//ClassOrInterfaceType[@Image = ancestor::ClassOrInterfaceDeclaration/@SimpleName]
]
//FieldDeclaration[@Static]
/VariableDeclarator
/MethodCall[@MethodName = "getLogger"]/AmbiguousName[@Name = "LoggerFactory"]
]]>
</value>
</property>
Expand Down Expand Up @@ -63,10 +60,10 @@
<property name="xpath">
<value>
<![CDATA[
//TypeDeclaration
//ClassBody/*
[
//FieldDeclaration/Type/ReferenceType/ClassOrInterfaceType[@Image='Optional'] or
//FormalParameter/Type/ReferenceType/ClassOrInterfaceType[@Image='Optional']
self::FieldDeclaration/ClassType[@SimpleName='Optional'] or
self::*/FormalParameters/FormalParameter/ClassType[@SimpleName='Optional']
]
]]>
</value>
Expand Down Expand Up @@ -111,7 +108,7 @@
<value>
<![CDATA[
//ImportDeclaration
[starts-with(Name/@Image, 'com.google.common.base.Optional')]
[starts-with(@ImportedName, 'com.google.common.base.Optional')]
]]>
</value>
</property>
Expand All @@ -134,12 +131,9 @@
<property name="xpath">
<value>
<![CDATA[
//ClassOrInterfaceDeclaration[@Public='true']
//ClassOrInterfaceBodyDeclaration
[MethodDeclaration[@Public='true' and @Void='false']/ResultType/Type/ReferenceType and
not(
Annotation/MarkerAnnotation/Name[@Image='Nullable'] or
Annotation/MarkerAnnotation/Name[@Image='Nonnull']
//MethodDeclaration[@EffectiveVisibility='public' and @Void=false() and ClassType and not(
./ModifierList/Annotation[@SimpleName='Nullable'] or
./ModifierList/Annotation[@SimpleName='Nonnull']
)]
]]>
</value>
Expand Down Expand Up @@ -182,68 +176,22 @@
message="Parameter type of public method is not annotated with @Nonnull or @Nullable."
class="net.sourceforge.pmd.lang.rule.xpath.XPathRule">
<description>
Parameter type of public method is not annotated with @Nonnull or @Nullable.
Missing @Nonnull or @Nullable annotation on parameter of public method or constructor.
This is beneficial to communicate a clear API contract and it allows for effective argument constraint validation in the IDE.
</description>
<priority>5</priority>
<properties>
<property name="xpath">
<value>
<![CDATA[
//ClassOrInterfaceDeclaration[@Public='true']
//ClassOrInterfaceBodyDeclaration
/MethodDeclaration[@Public='true']
/MethodDeclarator
//ClassBody
/*[@EffectiveVisibility='public']
/FormalParameters
/FormalParameter[
./Type/ReferenceType and
ClassType and
not(
./Annotation/MarkerAnnotation/Name[@Image='Nullable'] or
./Annotation/MarkerAnnotation/Name[@Image='Nonnull']
)]
]]>
</value>
</property>
</properties>
<example>
<![CDATA[
public interface SomeType {
// Okay
void method();
// Okay
void method( @Nullable Object parameter );
// Okay
void method( @Nonnull Object parameter, int primitive );
// Nullability annotation missing
void method( Object parameter );
}
]]>
</example>
</rule>
<rule name="NullAnnotationMissingOnPublicConstructorParameter" language="java"
message="Parameter type of public constructor is not annotated with @Nonnull or @Nullable."
class="net.sourceforge.pmd.lang.rule.xpath.XPathRule">
<description>
Parameter type of public constructor is not annotated with @Nonnull or @Nullable.
This is beneficial to communicate a clear API contract and it allows for effective argument constraint validation in the IDE.
</description>
<priority>5</priority>
<properties>
<property name="xpath">
<value>
<![CDATA[
//ClassOrInterfaceDeclaration[@Public='true']
//ClassOrInterfaceBodyDeclaration
/ConstructorDeclaration[@Public='true']
/FormalParameters
/FormalParameter[
./Type/ReferenceType and
not(
./Annotation/MarkerAnnotation/Name[@Image='Nullable'] or
./Annotation/MarkerAnnotation/Name[@Image='Nonnull']
./ModifierList/Annotation[@SimpleName='Nullable'] or
./ModifierList/Annotation[@SimpleName='Nonnull']
)]
]]>
</value>
Expand All @@ -267,56 +215,39 @@
]]>
</example>
</rule>
<rule name="PrematureInstantiationInElseBlock" language="java"
message="An instance is created in advance as part of an else block declaration."
class="net.sourceforge.pmd.lang.rule.xpath.XPathRule">
<description>
Do not unnecessarily compute code in advance, when it's not needed.
Avoid premature instantiation.
Instead of direct constructor call, use lambda notation or constructor reference, e.g. TargetType::new
</description>
<priority>2</priority>
<properties>
<property name="xpath">
<value>
<![CDATA[
//PrimarySuffix[@Image='orElse' or @Image='getOrElse']
/following-sibling::PrimarySuffix
/Arguments
/ArgumentList[count(./Expression)=1]
/Expression
/PrimaryExpression
/*
/AllocationExpression
]]>
</value>
</property>
</properties>
</rule>
<rule name="PrematureComputationOfElseBlock" language="java"
message="Else block of fluent-API is computed in advance."
class="net.sourceforge.pmd.lang.rule.xpath.XPathRule">
<description>
Do not unnecessarily compute code in advance, when it's not needed.
Instead of invoking the fallback function, reference it as lambda or function reference, e.g. this::fallback
Avoid premature instantiation.
Instead of direct constructor call, use lambda notation or constructor reference, e.g. TargetType::new
</description>
<priority>2</priority>
<properties>
<property name="xpath">
<value>
<![CDATA[
//PrimarySuffix[@Image='orElse' or @Image='getOrElse']
/following-sibling::PrimarySuffix[1]
/Arguments
/ArgumentList[count(./Expression)=1]
/Expression
/PrimaryExpression
/*
/Arguments
//MethodCall[@MethodName='orElse' or @MethodName='getOrElse']
/ArgumentList[not(LambdaExpression) and (.//MethodCall or .//ConstructorCall)]
]]>
</value>
</property>
</properties>
<example>
<![CDATA[
public class SomeType {
// Okay
Option.of(variable).getOrElse("foo");
Optional.of(variable).orElse(foo);
Optional.of(variable).orElseGet(() -> "foo" + bar());
// Not okay
Option.of(variable).getOrElse(bar());
Optional.of(variable).orElse("foo" + bar());
}
]]>
</example>
</rule>

<!-- The following "performance"-related rules are taken from here: https://pmd.github.io/latest/pmd_rules_java_performance.html -->
Expand Down Expand Up @@ -425,11 +356,8 @@
<![CDATA[
//IfStatement[
@Else=false() and
./Expression/PrimaryExpression/PrimaryPrefix/Name[starts-with(@Image, "log.is") and ends-with(@Image, "Enabled")] and
./Statement/Block[count(*)=1]/BlockStatement/Statement/StatementExpression/PrimaryExpression[
./PrimaryPrefix/Name[starts-with(@Image, "log.")] and
count(.//Arguments)=1
]
./MethodCall[starts-with(@MethodName, "is") and ends-with(@MethodName, "Enabled") and AmbiguousName[@Name="log"]] and
./Block[@Size=1]/ExpressionStatement/MethodCall[AmbiguousName[@Name="log"] and not(.//MethodCall) and not(.//ConstructorCall)]
]
]]>
</value>
Expand Down
2 changes: 2 additions & 0 deletions ...rc/main/java/com/sap/cloud/sdk/datamodel/metadata/generator/MetadataApiSpecificUsage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ static class Declaration
@Nullable
String value;

@Nonnull
@Override
public String toString()
{
final String typeUnqualified = getUnqualified(type);
Expand Down
3 changes: 3 additions & 0 deletions datamodel/fluent-result/src/main/java/com/sap/cloud/sdk/result/ElementName.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

import javax.annotation.Nonnull;

/**
* Annotation indicating the name of an element.
*/
Expand All @@ -21,5 +23,6 @@
*
* @return The identifiable name of the field.
*/
@Nonnull
String value();
}
1 change: 1 addition & 0 deletions ...ta-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataFormat.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ enum ODataFormat
@Nonnull
private final String httpAccept;

@Nonnull
@Override
public String toString()
{
Expand Down
4 changes: 4 additions & 0 deletions ...odata-core/src/main/java/com/sap/cloud/sdk/s4hana/datamodel/odata/adapter/ODataField.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

import javax.annotation.Nonnull;

import com.sap.cloud.sdk.typeconverter.TypeConverter;

/**
Expand All @@ -25,12 +27,14 @@
*
* @return The name of the corresponding OData property.
*/
@Nonnull
String odataName();

/**
* The converter to be used to convert between the domain and the exposed type of the annotated field.
*
* @return The type of the converter to use for the annotated field.
*/
@Nonnull
Class<? extends TypeConverter<?, ?>> converter() default IdentityConverter.class;
}
1 change: 1 addition & 0 deletions ...erator/src/main/java/com/sap/cloud/sdk/datamodel/openapi/generator/model/ApiMaturity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ public static ApiMaturity getValueOrDefault( @Nullable final String identifier )
BETA.identifier));
}

@Nonnull
@Override
public String toString()
{
Expand Down
1 change: 1 addition & 0 deletions s4hana/rfc/src/main/java/com/sap/cloud/sdk/s4hana/connectivity/rfc/ParameterKind.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ enum ParameterKind
* @deprecated This module will be discontinued, along with its classes and methods.
*/
@Deprecated
@Nonnull
@Override
public String toString()
{
Expand Down
3 changes: 3 additions & 0 deletions s4hana/rfc/src/main/java/com/sap/cloud/sdk/s4hana/connectivity/rfc/SoapNamespace.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

package com.sap.cloud.sdk.s4hana.connectivity.rfc;

import javax.annotation.Nonnull;

import lombok.AccessLevel;
import lombok.AllArgsConstructor;
import lombok.Setter;
Expand Down Expand Up @@ -31,6 +33,7 @@ public enum SoapNamespace
@Setter( AccessLevel.PUBLIC )
private String label;

@Nonnull
@Override
public String toString()
{
Expand Down
Loading

0 comments on commit 4b575ff

Please sign in to comment.