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

Issue #45: resolved many checkstyle violations #74

Merged
merged 1 commit into from
Feb 14, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions checkstyle-sonar-plugin/config/sevntu_suppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,17 @@

<suppressions>
<!-- START of legacy code, all violations will be resolved during transition to main project -->
<suppress checks=".*" files=".*[\\/]src[\\/]main[\\/]"/>
<suppress checks=".*" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="IllegalCatchExtended" files=".*[\\/]src[\\/]main[\\/]"/>
<suppress checks="ForbidAnnotation" files=".*[\\/]src[\\/]main[\\/]"/>

<suppress checks="ForbidAnnotation" files=".*[\\/]src[\\/]test[\\/]"/>
<!-- END of legacy code -->


<!-- parsing of xml imply reference attribute names, does not make sense to move all to special variables -->
<suppress checks="MultipleStringLiteralsExtended" files="CheckstyleProfileImporter\.java"/>

<!-- Tone down the checking for test code -->
<suppress checks="MultipleStringLiteralsExtended" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="ForbidCCommentsInMethods" files=".*[\\/]src[\\/]test[\\/]"/>
</suppressions>
22 changes: 4 additions & 18 deletions checkstyle-sonar-plugin/config/suppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,40 +7,26 @@
<suppressions>
<!-- START of legacy code, all violations will be resolved during transition to main project -->
<suppress checks="Javadoc*" files=".*[\\/]src[\\/]main[\\/]"/>
<suppress checks="FinalLocalVariable" files=".*[\\/]src[\\/]main[\\/]"/>
<suppress checks="ImportControl" files=".*[\\/]src[\\/]main[\\/]"/>
<suppress checks="DesignForExtension" files=".*[\\/]src[\\/]main[\\/]"/>
<suppress checks="WriteTag" files=".*[\\/]src[\\/]main[\\/]"/>
<suppress checks="DeclarationOrder" files=".*[\\/]src[\\/]main[\\/]"/>
<suppress checks="IllegalCatch" files=".*[\\/]src[\\/]main[\\/]"/>
<suppress checks="CatchParameterName" files=".*[\\/]src[\\/]main[\\/]"/>
<suppress checks="MemberName" files=".*[\\/]src[\\/]main[\\/]"/>
<suppress checks="InnerTypeLast" files=".*[\\/]src[\\/]main[\\/]"/>
<suppress checks="ReturnCount" files=".*[\\/]src[\\/]main[\\/]"/>
<suppress checks="ParameterName" files=".*[\\/]src[\\/]main[\\/]"/>

<suppress checks="FinalLocalVariable" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="HiddenField" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="VariableDeclarationUsageDistance" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="DesignForExtension" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="ImportControl" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="ClassDataAbstractionCoupling" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="CyclomaticComplexity" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="NPathComplexity" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="CatchParameterName" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="MemberName" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="AnonInnerLength" files=".*[\\/]src[\\/]test[\\/]"/>
<!-- END of legacy code -->


<!-- parsing of xml imply reference attrubute names, does not make sense to move all to special variables -->
<!-- parsing of xml imply reference attribute names, does not make sense to move all to special variables -->
<suppress checks="MultipleStringLiterals" files="CheckstyleProfileImporter\.java"/>
<!-- messing test code with such optimization does not make sense , readability will decrease -->
<suppress checks="MultipleStringLiterals" files=".*[\\/]src[\\/]test[\\/]"/>

<!-- Tone down the checking for test code -->
<suppress checks="NPathComplexity|CyclomaticComplexity" files=".*[\\/]ChecksTest\.java"/>
<suppress checks="Javadoc" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="WriteTag" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="AvoidStaticImport" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="MagicNumber" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="AnonInnerLength" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="ClassDataAbstractionCoupling" files=".*[\\/]src[\\/]test[\\/]"/>
</suppressions>
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@ public class CheckstyleAuditListener implements AuditListener, BatchExtension {
private static final Logger LOG = LoggerFactory.getLogger(CheckstyleAuditListener.class);

private final RuleFinder ruleFinder;
private final FileSystem fs;
private final FileSystem fileSystem;
private final ResourcePerspectives perspectives;
private InputFile currentResource;

public CheckstyleAuditListener(RuleFinder ruleFinder, FileSystem fs,
public CheckstyleAuditListener(RuleFinder ruleFinder, FileSystem fileSystem,
ResourcePerspectives perspectives) {
this.ruleFinder = ruleFinder;
this.fs = fs;
this.fileSystem = fileSystem;
this.perspectives = perspectives;
}

Expand All @@ -76,19 +76,19 @@ public void fileFinished(AuditEvent event) {

@Override
public void addError(AuditEvent event) {
String ruleKey = getRuleKey(event);
final String ruleKey = getRuleKey(event);
if (ruleKey != null) {
String message = getMessage(event);
final String message = getMessage(event);
// In Checkstyle 5.5 exceptions are reported as an events from
// TreeWalker
if ("com.puppycrawl.tools.checkstyle.TreeWalker".equals(ruleKey)) {
LOG.warn("{} : {}", event.getFileName(), message);
}
initResource(event);
Issuable issuable = perspectives.as(Issuable.class, currentResource);
Rule rule = ruleFinder.findByKey(CheckstyleConstants.REPOSITORY_KEY, ruleKey);
final Issuable issuable = perspectives.as(Issuable.class, currentResource);
final Rule rule = ruleFinder.findByKey(CheckstyleConstants.REPOSITORY_KEY, ruleKey);
if (rule != null && issuable != null) {
IssueBuilder issueBuilder = issuable.newIssueBuilder().ruleKey(rule.ruleKey())
final IssueBuilder issueBuilder = issuable.newIssueBuilder().ruleKey(rule.ruleKey())
.message(message).line(getLineId(event));
issuable.addIssue(issueBuilder.build());
}
Expand All @@ -97,8 +97,9 @@ public void addError(AuditEvent event) {

private void initResource(AuditEvent event) {
if (currentResource == null) {
String absoluteFilename = event.getFileName();
currentResource = fs.inputFile(fs.predicates().hasAbsolutePath(absoluteFilename));
final String absoluteFilename = event.getFileName();
currentResource = fileSystem.inputFile(fileSystem.predicates().hasAbsolutePath(
absoluteFilename));
}
}

Expand All @@ -108,16 +109,17 @@ static String getRuleKey(AuditEvent event) {
try {
key = event.getModuleId();
}
catch (Exception e) {
LOG.warn("AuditEvent is created incorrectly. Exception happen during getModuleId()", e);
catch (Exception ex) {
LOG.warn("AuditEvent is created incorrectly. Exception happen during getModuleId()",
ex);
}
if (StringUtils.isBlank(key)) {
try {
key = event.getSourceName();
}
catch (Exception e) {
catch (Exception ex) {
LOG.warn("AuditEvent is created incorrectly."
+ "Exception happen during getSourceName()", e);
+ "Exception happen during getSourceName()", ex);
}
}
return key;
Expand All @@ -129,8 +131,8 @@ static String getMessage(AuditEvent event) {
return event.getMessage();

}
catch (Exception e) {
LOG.warn("AuditEvent is created incorrectly. Exception happen during getMessage()", e);
catch (Exception ex) {
LOG.warn("AuditEvent is created incorrectly. Exception happen during getMessage()", ex);
return null;
}
}
Expand All @@ -139,15 +141,15 @@ static String getMessage(AuditEvent event) {
static Integer getLineId(AuditEvent event) {
Integer result = null;
try {
int line = event.getLine();
final int line = event.getLine();
// checkstyle returns 0 if there is no relation to a file content,
// but we use null
if (line != 0) {
result = line;
}
}
catch (Exception e) {
LOG.warn("AuditEvent is created incorrectly. Exception happen during getLine()", e);
catch (Exception ex) {
LOG.warn("AuditEvent is created incorrectly. Exception happen during getLine()", ex);
}
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@
import com.puppycrawl.tools.checkstyle.api.Configuration;

public class CheckstyleConfiguration implements BatchExtension {
public static final String PROPERTY_GENERATE_XML = "sonar.checkstyle.generateXml";

private static final Logger LOG = LoggerFactory.getLogger(CheckstyleConfiguration.class);
public static final String PROPERTY_GENERATE_XML = "sonar.checkstyle.generateXml";

private final CheckstyleProfileExporter confExporter;
private final RulesProfile profile;
Expand All @@ -65,7 +65,7 @@ public CheckstyleConfiguration(Settings conf, CheckstyleProfileExporter confExpo
}

public File getXmlDefinitionFile() {
File xmlFile = new File(fileSystem.workDir(), "checkstyle.xml");
final File xmlFile = new File(fileSystem.workDir(), "checkstyle.xml");
try (Writer writer = new OutputStreamWriter(new FileOutputStream(xmlFile, false),
StandardCharsets.UTF_8)) {

Expand All @@ -74,19 +74,19 @@ public File getXmlDefinitionFile() {
return xmlFile;

}
catch (IOException e) {
catch (IOException ex) {
throw new IllegalStateException("Fail to save the Checkstyle configuration to "
+ xmlFile.getPath(), e);
+ xmlFile.getPath(), ex);

}
}

public List<File> getSourceFiles() {
FilePredicates predicates = fileSystem.predicates();
Iterable<File> files = fileSystem.files(predicates.and(
final FilePredicates predicates = fileSystem.predicates();
final Iterable<File> files = fileSystem.files(predicates.and(
predicates.hasLanguage(CheckstyleConstants.JAVA_KEY),
predicates.hasType(InputFile.Type.MAIN)));
List<File> fileList = new ArrayList<>();
final List<File> fileList = new ArrayList<>();
for (File file : files) {
fileList.add(file);
}
Expand All @@ -101,10 +101,10 @@ public File getTargetXmlReport() {
}

public Configuration getCheckstyleConfiguration() throws CheckstyleException {
File xmlConfig = getXmlDefinitionFile();
final File xmlConfig = getXmlDefinitionFile();

LOG.info("Checkstyle configuration: {}", xmlConfig.getAbsolutePath());
Configuration configuration = toCheckstyleConfiguration(xmlConfig);
final Configuration configuration = toCheckstyleConfiguration(xmlConfig);
defineCharset(configuration);
return configuration;
}
Expand All @@ -123,10 +123,11 @@ private void defineCharset(Configuration configuration) {
}

private void defineModuleCharset(Configuration module) {
if (("Checker".equals(module.getName()) || "com.puppycrawl.tools.checkstyle.Checker"
.equals(module.getName())) && module instanceof DefaultConfiguration) {
Charset charset = getCharset();
String charsetName = charset.name();
if (module instanceof DefaultConfiguration
&& ("Checker".equals(module.getName())
|| "com.puppycrawl.tools.checkstyle.Checker".equals(module.getName()))) {
final Charset charset = getCharset();
final String charsetName = charset.name();
LOG.info("Checkstyle charset: {}", charsetName);
((DefaultConfiguration) module).addAttribute("charset", charsetName);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ public CheckstyleExecutor(CheckstyleConfiguration configuration,
*/
public void execute() {

Locale initialLocale = Locale.getDefault();
final Locale initialLocale = Locale.getDefault();
Locale.setDefault(Locale.ENGLISH);
ClassLoader initialClassLoader = Thread.currentThread().getContextClassLoader();
final ClassLoader initialClassLoader = Thread.currentThread().getContextClassLoader();
Thread.currentThread().setContextClassLoader(PackageNamesLoader.class.getClassLoader());
URLClassLoader projectClassloader = createClassloader();
final URLClassLoader projectClassloader = createClassloader();
try {
executeWithClassLoader(projectClassloader);
}
Expand All @@ -79,16 +79,16 @@ public void execute() {
}

private void executeWithClassLoader(URLClassLoader projectClassloader) {
TimeProfiler profiler = new TimeProfiler().start("Execute Checkstyle "
final TimeProfiler profiler = new TimeProfiler().start("Execute Checkstyle "
+ CheckstyleVersion.getVersion());
Checker checker = new Checker();
final Checker checker = new Checker();
OutputStream xmlOutput = null;
try {
checker.setClassLoader(projectClassloader);
checker.setModuleClassLoader(Thread.currentThread().getContextClassLoader());
checker.addListener(listener);

File xmlReport = configuration.getTargetXmlReport();
final File xmlReport = configuration.getTargetXmlReport();
if (xmlReport != null) {
LOG.info("Checkstyle output report: {}", xmlReport.getAbsolutePath());
xmlOutput = FileUtils.openOutputStream(xmlReport);
Expand All @@ -102,8 +102,8 @@ private void executeWithClassLoader(URLClassLoader projectClassloader) {
profiler.stop();

}
catch (Exception e) {
throw new IllegalStateException("Can not execute Checkstyle", e);
catch (Exception ex) {
throw new IllegalStateException("Can not execute Checkstyle", ex);
}
finally {
checker.destroy();
Expand All @@ -118,8 +118,8 @@ static void close(Closeable closeable) {
try {
closeable.close();
}
catch (IOException e) {
throw new IllegalStateException("failed to close object", e);
catch (IOException ex) {
throw new IllegalStateException("failed to close object", ex);
}
}

Expand All @@ -128,15 +128,15 @@ URL getUrl(URI uri) {
try {
return uri.toURL();
}
catch (MalformedURLException e) {
catch (MalformedURLException ex) {
throw new IllegalStateException("Fail to create the project classloader. "
+ "Classpath element is invalid: " + uri, e);
+ "Classpath element is invalid: " + uri, ex);
}
}

private URLClassLoader createClassloader() {
Collection<File> classpathElements = javaResourceLocator.classpath();
List<URL> urls = new ArrayList<>(classpathElements.size());
final Collection<File> classpathElements = javaResourceLocator.classpath();
final List<URL> urls = new ArrayList<>(classpathElements.size());
for (File file : classpathElements) {
urls.add(getUrl(file.toURI()));
}
Expand Down
Loading