Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Error Prone code style verification #632

Merged
merged 40 commits into from
Oct 31, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
5ae71f6
Add Error Prone code style verification
vlsi Oct 18, 2020
43217ec
Remove LoggingManager class (it has been deprecated since JMeter 3.2)
vlsi Oct 18, 2020
797f9e4
Use UTF-8 encoding in BeanShellClient instead of platform-specific one
vlsi Oct 18, 2020
f6b1bfa
Add JavaDoc summaries to RandomVariableConfig, CounterConfig, JavaSam…
vlsi Oct 18, 2020
f5e3d7a
Suppress error-prone errors for junit-sample
vlsi Oct 18, 2020
ba94b52
Replace obsolete Vector and Hashtable with ArrayList and HashMap
vlsi Oct 18, 2020
b3b2eec
Replace LinkedList with ArrayList
vlsi Oct 18, 2020
db768c0
Remove uses of deprecated APIs
vlsi Oct 18, 2020
77d5d1b
Remove excessive parenthesis
vlsi Oct 18, 2020
73b925c
Avoid unsafe reflection cast
vlsi Oct 18, 2020
f5c64e1
Avoid use of Enumeration in ClassFinder
vlsi Oct 18, 2020
e8fbfa7
Fixup JavaDocs
vlsi Oct 18, 2020
9c92e99
Make constructors of abstract classes protected rather than public
vlsi Oct 18, 2020
7e42131
Avoid import Map.Entry to avoid amboguity
vlsi Oct 18, 2020
93e27d7
Use lower-case field names for non-static fields in StatCalculator
vlsi Oct 18, 2020
dc4bdc0
Make enums immutable
vlsi Oct 18, 2020
63d791c
Suppresss MissingSummary and EmptyBlockTag for now
vlsi Oct 18, 2020
599fcc9
Suppress deprecation warnings for TestResultAction
vlsi Oct 18, 2020
2fde32b
Suppress deprecation warnings in SamplerMetricFixedModeTest
vlsi Oct 18, 2020
80c3914
Suppress DefaultCharset usage: either suppress the warning or use a c…
vlsi Oct 18, 2020
e78352a
Fix InconsistentCapitalization warnings
vlsi Oct 18, 2020
369f651
Suppress JdkObsolete warnings for Enumeration
vlsi Oct 18, 2020
deb89ba
Remove unused variables
vlsi Oct 18, 2020
3d133a8
Suppress JdkObsolete warnings for usages of Date
vlsi Oct 18, 2020
5f84f1e
Add missing @Override annotations
vlsi Oct 18, 2020
b1255aa
Mark empty catch blocks with a comment
vlsi Oct 18, 2020
d756670
Suppress FutureReturnValueIgnored
vlsi Oct 18, 2020
bb34632
Suppress Thread.yield usage
vlsi Oct 18, 2020
c441e5b
Suppress JavaTimeDefaultTimeZone
vlsi Oct 18, 2020
ac19a29
Inline format specifiers when they are used only once
vlsi Oct 18, 2020
ecf72b7
Resolve MixedMutabilityReturnType: make the returned lists unmodifiable
vlsi Oct 18, 2020
8f5ca8d
Suppress UnnecessaryAnonymousClass as the only two warnings seem to b…
vlsi Oct 18, 2020
8ae3458
Resolve SynchronizeOnNonFinalField
vlsi Oct 18, 2020
9d7b694
Suppress warnings for Hashtable usage when the class is required for …
vlsi Oct 18, 2020
a4c1ab8
Move .lock() statements out of try to avoid accidetal release if lock…
vlsi Oct 18, 2020
ee0aead
Suppress StaticAssignmentInConstructor in Summarizer
vlsi Oct 18, 2020
7e2a47d
Suppress JdkObsolete when using appendReplacement(StringBuffer,...)
vlsi Oct 18, 2020
5fad27c
Suppress JavaLangClash for ThreadGroup
vlsi Oct 18, 2020
ebe65b8
Avoid NarrowingCompoundAssignment warning
vlsi Oct 18, 2020
150a275
Make members of final classes package-private
vlsi Oct 18, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
18 changes: 18 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,21 @@ jobs:
job-id: jdk14
multi-cache-enabled: false
arguments: --scan --no-parallel build -x distTar -x distTarSource -Dskip.test_TestDNSCacheManager.testWithCustomResolverAnd1Server=true

errorprone:
name: 'Error Prone (JDK 11)'
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
with:
fetch-depth: 50
- name: 'Set up JDK 11'
uses: actions/setup-java@v1
with:
java-version: 11
- uses: burrunan/gradle-cache-action@v1
name: Test
with:
job-id: errprone
multi-cache-enabled: false
arguments: --scan --no-parallel --no-daemon -PenableErrorprone classes
39 changes: 39 additions & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import com.github.vlsi.gradle.git.dsl.gitignore
import com.github.vlsi.gradle.properties.dsl.lastEditYear
import com.github.vlsi.gradle.properties.dsl.props
import com.github.vlsi.gradle.release.RepositoryType
import net.ltgt.gradle.errorprone.errorprone
import org.ajoberstar.grgit.Grgit
import org.gradle.api.tasks.testing.logging.TestExceptionFormat
import org.sonarqube.gradle.SonarQubeProperties
Expand All @@ -36,6 +37,7 @@ plugins {
id("org.nosphere.apache.rat")
id("com.github.autostyle")
id("com.github.spotbugs")
id("net.ltgt.errorprone") apply false
id("org.sonarqube")
id("com.github.vlsi.crlf")
id("com.github.vlsi.gradle-extensions")
Expand Down Expand Up @@ -147,9 +149,11 @@ val jacocoEnabled by extra {

// Do not enable spotbugs by default. Execute it only when -Pspotbugs is present
val enableSpotBugs = props.bool("spotbugs", default = false)
val enableErrorprone by props()
val ignoreSpotBugsFailures by props()
val skipCheckstyle by props()
val skipAutostyle by props()
val werror by props(true) // treat javac warnings as errors
// Allow to skip building source/binary distributions
val skipDist by extra {
boolProp("skipDist") ?: false
Expand Down Expand Up @@ -381,6 +385,37 @@ allprojects {
dependsOn("checkstyleAll")
}
}

if (enableErrorprone) {
apply(plugin = "net.ltgt.errorprone")
dependencies {
"errorprone"("com.google.errorprone:error_prone_core:${"errorprone".v}")
"annotationProcessor"("com.google.guava:guava-beta-checker:1.0")
}
tasks.withType<JavaCompile>().configureEach {
options.compilerArgs.addAll(listOf("-Xmaxerrs", "10000", "-Xmaxwarns", "10000"))
options.errorprone {
disableWarningsInGeneratedCode.set(true)
errorproneArgs.add("-XepExcludedPaths:.*/javacc/.*")
disable(
"ComplexBooleanConstant",
"EqualsGetClass",
"OperatorPrecedence",
"MutableConstantField",
// "ReferenceEquality",
"SameNameButDifferent",
"TypeParameterUnusedInFormals"
)
// Analyze issues, and enable the check
disable(
"MissingSummary",
"EmptyBlockTag",
"BigDecimalEquals",
"StringSplitter"
)
}
}
}
}
plugins.withId("groovy") {
if (!skipAutostyle) {
Expand Down Expand Up @@ -468,6 +503,10 @@ allprojects {
tasks {
withType<JavaCompile>().configureEach {
options.encoding = "UTF-8"
options.compilerArgs.add("-Xlint:deprecation")
if (werror) {
options.compilerArgs.add("-Werror")
}
}
withType<ProcessResources>().configureEach {
filteringCharset = "UTF-8"
Expand Down
12 changes: 12 additions & 0 deletions checksum.xml
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,17 @@
<trusted-key id='105cb91cac2aee0e' group='com.github.autostyle' />
<trusted-key id='3c0a8f4744f37328' group='com.github.ben-manes.caffeine' />
<trusted-key id='8ea48d105232855d' group='com.github.jknack' />
<trusted-key id='218fa0f6a941a037' group='com.github.kevinstern' />
<trusted-key id='461a804f2609fd89' group='com.github.shyiko.klob' />
<trusted-key id='1756b920eecf0e90' group='com.github.spotbugs' />
<trusted-key id='a5def5a76f94a471' group='com.github.spotbugs' />
<trusted-key id='78fcd238c26ea995' group='com.github.tomakehurst' />
<trusted-key id='105cb91cac2aee0e' group='com.github.vlsi.gradle' />
<trusted-key id='254e9c64c264c176' group='com.github.weisj' />
<trusted-key id='5e1f79a7c298661e' group='com.google.auto' />
<trusted-key id='c51e6cbc7ff46f0b' group='com.google.auto.service' />
<trusted-key id='b0f3710fa64900e7' group='com.google.auto.value' />
<trusted-key id='353a436e043e3145' group='com.google.code.findbugs' />
<trusted-key id='59a252fb1199d873' group='com.google.code.findbugs' />
<trusted-key id='7a01b0f236e5430f' group='com.google.code.gson' />
<trusted-key id='1669c4bb543e0445' group='com.google.errorprone' />
Expand All @@ -28,7 +33,9 @@
<trusted-key id='abe9f3126bb741c1' group='com.google.guava' />
<trusted-key id='f6d4a1d411e9d1ae' group='com.google.guava' />
<trusted-key id='29579f18fa8fd93b' group='com.google.j2objc' />
<trusted-key id='a7764f502a938c99' group='com.google.protobuf' />
<trusted-key id='7eb97d110dfadd60' group='com.googlecode.concurrent-trees' />
<trusted-key id='476634a4694e716a' group='com.googlecode.java-diff-utils' />
<trusted-key id='72475fd306b9cab7' group='com.googlecode.javaewah' />
<trusted-key id='e57428da9e879e7d' group='com.helger' />
<trusted-key id='3684155e9365c30e' group='com.jayway.jsonpath' />
Expand Down Expand Up @@ -144,6 +151,7 @@
<trusted-key id='7c7d8456294423ba' group='org.objenesis' />
<trusted-key id='85911f425ec61b51' group='org.opentest4j' />
<trusted-key id='5f69ad087600b22c' group='org.ow2.asm' />
<trusted-key id='2be5d98f751f4136' group='org.pcollections' />
<trusted-key id='1364c5e2df3e99c5' group='org.reactivestreams' />
<trusted-key id='2c7b12f2a511e325' group='org.slf4j' />
<trusted-key id='cfca4a29d26468de' group='org.sonarsource.scanner.api' />
Expand All @@ -152,6 +160,7 @@
<trusted-key id='af0a94402ea0a67f' group='org.spockframework' />
<trusted-key id='9a2c7a98e457c53d' group='org.springframework' />
<trusted-key id='e4b53844e02b191b' group='org.swinglabs' />
<trusted-key id='72385ff0af338d52' group='org.threeten' />
<trusted-key id='a2115ae15f6b8b72' group='org.xmlunit' />
<trusted-key id='9ff25980f5ba7e4f' group='xalan' />
<trusted-key id='858fc4c4f43856a3' group='xerces' />
Expand Down Expand Up @@ -188,6 +197,9 @@
<dependency group='net.jcip' module='jcip-annotations' version='1.0'>
<sha512>CB312B3F571D91EF183C119D878F50464FFD97F853B7311CBA386463F295E8B7B3A5A89ED4269A045CACD5AA7CB4C803D4882854A0FDDEFA9BBC28C72AA6C786</sha512>
</dependency>
<dependency group='net.ltgt.gradle' module='gradle-errorprone-plugin' version='1.2.1'>
<sha512>DC1FA477AE67F94B7CA427F1F88FD3C56E562EB3ED92A3CBC4CC116258EB3856F4B53C99506FDC279F6983EAE74BA21271E4A152F5DB8B41392FFCBB6B087C78</sha512>
</dependency>
<dependency group='net.sf.ezmorph' module='ezmorph' version='1.0.6'>
<sha512>16D30BE564723B74F312B4E7D06F349370FB6726B3162778C869CD723ECA2A40C4972C2757B3E107E1820CEC0D70B0BD2B96EFCD466518FC64495F7AEF97967A</sha512>
</dependency>
Expand Down
4 changes: 2 additions & 2 deletions extras/startup.bsh
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ bsh.system.shutdownOnExit = false;
print("Startup script running");

import org.apache.jmeter.util.JMeterUtils;
import org.apache.jorphan.logging.LoggingManager;

getprop(p){// get a JMeter property
return JMeterUtils.getPropDefault(p,"");
Expand All @@ -49,7 +48,8 @@ print(p + " = " + getprop(p));
}

loglevel(String priority, String category){
LoggingManager.setPriority(priority, category);
// Previous implementation delegated the call to LoggingManager which
// was deprecated since JMeter 3.2
}

logdebug(String text){
Expand Down
2 changes: 2 additions & 0 deletions gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ jmeter.version=5.3.1

# Tools
checkstyle.version=8.35
errorprone.version=2.4.0
jacoco.version=0.8.5
spotbugs.version=4.1.2
velocity.version=1.7
Expand All @@ -44,6 +45,7 @@ com.github.spotbugs.version=4.5.0
com.github.vlsi.checksum-dependency.sha512=4D1A76F38F327CEA0C723D9BDD9ABFE16933769052F47BCECD555DDD1A6CD0A9C21E3CC8F9E1B92780F9B443070D4844889EE9ECB0690D30E50AAB085096D8E1
com.github.vlsi.checksum-dependency.version=1.70
com.github.vlsi.vlsi-release-plugins.version=1.70
net.ltgt.errorprone.version=1.2.1
org.jetbrains.gradle.plugin.idea-ext.version=0.5
org.nosphere.apache.rat.version=0.7.0
org.sonarqube.version=3.0
Expand Down
1 change: 1 addition & 0 deletions settings.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pluginManagement {
idv("com.github.vlsi.gradle-extensions", "com.github.vlsi.vlsi-release-plugins")
idv("com.github.vlsi.ide", "com.github.vlsi.vlsi-release-plugins")
idv("com.github.vlsi.stage-vote-release", "com.github.vlsi.vlsi-release-plugins")
idv("net.ltgt.errorprone")
idv("org.jetbrains.gradle.plugin.idea-ext")
idv("org.nosphere.apache.rat")
idv("org.sonarqube")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@

package org.apache.jmeter.util;

import java.io.FileReader;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.OutputStream;
import java.net.Socket;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Paths;

// N.B. Do not call any JMeter methods; the jar is standalone

Expand Down Expand Up @@ -65,7 +67,7 @@ public static void main(String [] args) throws Exception{
sendLine("};", os);

int b;
try (InputStreamReader fis = new FileReader(file)) {
try (BufferedReader fis = Files.newBufferedReader(Paths.get(file))) {
while ((b = fis.read()) != -1) {
os.write(b);
}
Expand All @@ -80,7 +82,7 @@ public static void main(String [] args) throws Exception{
private static void sendLine( String line, OutputStream outPipe )
throws IOException
{
outPipe.write( line.getBytes() ); // TODO - charset?
outPipe.write(line.getBytes(StandardCharsets.UTF_8));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure about the potential breaks this might introduce ?

Copy link
Collaborator Author

@vlsi vlsi Oct 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe there's a movement to use UTF-8 by default.

Of course, we can make it configurable, however, I believe, most of the times UTF-8 by default would be better (less chances to corrupt data) than use platform-default encoding.

outPipe.flush();
}

Expand All @@ -93,6 +95,7 @@ public SockRead(InputStream _is) {
}

@Override
@SuppressWarnings("CatchAndPrintStackTrace")
public void run(){
System.out.println("Reading responses from server ...");
int x = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
package org.apache.jmeter.assertions;

import java.io.Serializable;
import java.util.ArrayList;
import java.util.Collection;
import java.util.LinkedList;
import java.util.List;

import org.apache.jmeter.engine.event.LoopIterationEvent;
Expand Down Expand Up @@ -170,7 +170,7 @@ private String filterString(final String content) {

@Override
public void iterationStart(LoopIterationEvent iterEvent) {
responses = new LinkedList<>();
responses = new ArrayList<>();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.FileWriter;
import java.io.IOException;
import java.io.PrintWriter;
import java.io.Serializable;
import java.io.StringWriter;
import java.io.Writer;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.text.MessageFormat;

import org.apache.commons.lang3.StringUtils;
Expand Down Expand Up @@ -191,7 +193,7 @@ private void writeOutput(String inOutput) {

// check if filename defined
if (StringUtils.isNotBlank(filename)) {
try (FileWriter writer = new FileWriter(filename, false)){
try (Writer writer = Files.newBufferedWriter(Paths.get(filename))) {
// write to file
writer.write(inOutput);
log.debug("writeOutput() -> output successfully written to file: {}", filename);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
import java.io.Serializable;
import java.io.StringReader;

import javax.xml.parsers.ParserConfigurationException;
import javax.xml.parsers.SAXParserFactory;

import org.apache.jmeter.samplers.SampleResult;
import org.apache.jmeter.testelement.AbstractTestElement;
import org.apache.jmeter.testelement.ThreadListener;
Expand All @@ -29,7 +32,6 @@
import org.xml.sax.InputSource;
import org.xml.sax.SAXException;
import org.xml.sax.XMLReader;
import org.xml.sax.helpers.XMLReaderFactory;

/**
* Checks if the result is a well-formed XML content using {@link XMLReader}
Expand All @@ -45,10 +47,12 @@ public class XMLAssertion extends AbstractTestElement implements Serializable, A
@Override
protected XMLReader initialValue() {
try {
XMLReader reader = XMLReaderFactory.createXMLReader();
XMLReader reader = SAXParserFactory.newInstance()
.newSAXParser()
.getXMLReader();
reader.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
return reader;
} catch (SAXException e) {
} catch (SAXException | ParserConfigurationException e) {
log.error("Error initializing XMLReader in XMLAssertion", e);
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,21 +219,21 @@ private void initVars(FileServer server, final JMeterContext context, String del
trimVarNames(vars);
}

private void setAlias(final JMeterContext context, String fileName) {
private void setAlias(final JMeterContext context, String alias) {
String mode = getShareMode();
int modeInt = CSVDataSetBeanInfo.getShareModeAsInt(mode);
switch(modeInt){
case CSVDataSetBeanInfo.SHARE_ALL:
alias = fileName;
this.alias = alias;
break;
case CSVDataSetBeanInfo.SHARE_GROUP:
alias = fileName+"@"+System.identityHashCode(context.getThreadGroup());
this.alias = alias + "@" + System.identityHashCode(context.getThreadGroup());
break;
case CSVDataSetBeanInfo.SHARE_THREAD:
alias = fileName+"@"+System.identityHashCode(context.getThread());
this.alias = alias + "@" + System.identityHashCode(context.getThread());
break;
default:
alias = fileName+"@"+mode; // user-specified key
this.alias = alias + "@" + mode; // user-specified key
break;
}
}
Expand Down
Loading