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

Static code analyzer improvement #145

Merged
merged 8 commits into from
Mar 30, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 1 addition & 3 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ jobs:
set -x
./scripts/enforce-issue-number-for-todos
./scripts/enforce-version-bump
./scripts/prohibit-java-collections
./scripts/prohibit-trailing-whitespace
./scripts/prohibit-wildcard-imports
./scripts/specify-snapshot-revision

- run:
Expand All @@ -33,7 +31,7 @@ jobs:
command: ./gradlew javadoc
- run:
name: Run static code analyzer
command: ./gradlew spotbugsMain
command: ./gradlew checkstyleMain --warn

- run:
name: Compile tests
Expand Down
17 changes: 17 additions & 0 deletions backendApi/config/checkstyle/checkstyle.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
"-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
"https://checkstyle.org/dtds/configuration_1_3.dtd" [
<!ENTITY checkerModuleDirectives SYSTEM "../../../config/checkstyle/checker_module_directives.xml">
<!ENTITY treeWalkerModuleDirectives SYSTEM "../../../config/checkstyle/tree_walker_module_directives.xml">
<!ENTITY prohibitJavaCollections SYSTEM "../../../config/checkstyle/prohibit_java_collections.xml">
]>

<module name="Checker">
&checkerModuleDirectives;
micpiotrowski marked this conversation as resolved.
Show resolved Hide resolved

<module name="TreeWalker">
&treeWalkerModuleDirectives;
&prohibitJavaCollections;
</module>
</module>
micpiotrowski marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ private Optional<IGitCoreLocalBranch> getCoreBranchFromName(String branchName) {
}

@AllArgsConstructor(staticName = "of")
static private class GitMacheteBranchData {
private static class GitMacheteBranchData {
final IGitMacheteCommit pointedCommit;
final List<IGitMacheteCommit> commits;
final SyncToOriginStatus syncToOriginStatus;
Expand Down
17 changes: 17 additions & 0 deletions branchLayoutApi/config/checkstyle/checkstyle.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
"-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
"https://checkstyle.org/dtds/configuration_1_3.dtd" [
<!ENTITY checkerModuleDirectives SYSTEM "../../../config/checkstyle/checker_module_directives.xml">
<!ENTITY treeWalkerModuleDirectives SYSTEM "../../../config/checkstyle/tree_walker_module_directives.xml">
<!ENTITY prohibitJavaCollections SYSTEM "../../../config/checkstyle/prohibit_java_collections.xml">
]>

<module name="Checker">
&checkerModuleDirectives;

<module name="TreeWalker">
&treeWalkerModuleDirectives;
&prohibitJavaCollections;
</module>
</module>
micpiotrowski marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ public void save(IBranchLayout branchLayout, boolean backupOldFile) throws IOExc
var lines = printBranchesOntoStringList(branchLayout.getRootBranches(), 0);

if (backupOldFile) {
var pathToBackupFile = path.getParent().resolve(path.getFileName() + "~");
Path parentDir = path.getParent();
assert parentDir != null : "Can't get parent dir of branch relation file";
var pathToBackupFile = parentDir.resolve(path.getFileName() + "~");
Files.copy(path, pathToBackupFile, StandardCopyOption.REPLACE_EXISTING);
}

Expand Down
56 changes: 36 additions & 20 deletions build.gradle
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
plugins {
id 'org.gradle.checkstyle'
id 'org.gradle.java-library'

id 'com.diffplug.gradle.spotless' version '3.27.2' apply false
id 'com.github.spotbugs' version '3.0.0' apply false
id 'io.freefair.lombok' version '4.1.6' apply false
id 'org.checkerframework' version '0.4.13' apply false
id 'org.jetbrains.intellij' version '0.4.17' apply false
Expand All @@ -26,20 +26,19 @@ task resolveDependencies {
}
}

subprojects {
def PLUGIN_VERSION = '0.0.39-SNAPSHOT'

group 'com.virtuslab'
version PLUGIN_VERSION

allprojects {
// This is only needed as workaround for resolveDependencies task. Otherwise it fails when resolve dependency for checkstyle
micpiotrowski marked this conversation as resolved.
Show resolved Hide resolved
repositories {
jcenter()
mavenCentral()
// For spotbugs
maven {
url 'https://plugins.gradle.org/m2/'
}
}
}

subprojects {
def PLUGIN_VERSION = '0.0.40-SNAPSHOT'

group 'com.virtuslab'
version PLUGIN_VERSION

apply plugin: 'org.gradle.java-library'
sourceCompatibility = 11
Expand All @@ -66,13 +65,28 @@ subprojects {
}
}

apply plugin: 'com.github.spotbugs'
spotbugs {
apply plugin: 'checkstyle'
checkstyle {
toolVersion '8.30'

// ignoreFailures here because we want to set conditions that will cause build fail in violations plugin
PawelLipski marked this conversation as resolved.
Show resolved Hide resolved
ignoreFailures = true
effort = 'min'
showProgress = true
reportLevel = 'low'
ignoreFailures = false
checkstyleTest.enabled = false

// This enabled as to "importing" subconfig files to main file (DTD ENTITY feature)
micpiotrowski marked this conversation as resolved.
Show resolved Hide resolved
System.setProperty('checkstyle.enableExternalDtdLoad', 'true')

configProperties = [rootCheckstyleConfigDir: "${rootProject.projectDir}/config/checkstyle"]

def checkstyleRelativeConfigPath = 'config/checkstyle/checkstyle.xml'

def subprojectConfigFile = new File(projectDir, checkstyleRelativeConfigPath)
// Check if module-specific config file exist. If exist then apply
micpiotrowski marked this conversation as resolved.
Show resolved Hide resolved
if(subprojectConfigFile.isFile()) {
configFile = subprojectConfigFile;
} else {
configFile = new File(rootProject.projectDir, checkstyleRelativeConfigPath)
}
}

apply plugin: 'se.bjurr.violations.violations-gradle-plugin'
Expand All @@ -93,11 +107,13 @@ subprojects {
// This is mandatory regardless of if you want to report violations between revisions or the entire repo.
// Many more formats available, see: https://github.com/tomasbjerre/violations-lib
violations = [
['FINDBUGS', buildDir.path, '.*/spotbugs/.*\\.xml\$', 'Spotbugs']
['CHECKSTYLE', buildDir.path, '.*/checkstyle/.*\\.xml\$', 'Checkstyle']
]
}
// Run violations task after each spotbugsMain run
spotbugsMain.finalizedBy(violations)

// Run violations task after each checkstyleMain run
checkstyleMain.finalizedBy(violations)


ext {
guiceVersion = '4.2.2'
Expand Down
3 changes: 3 additions & 0 deletions config/checkstyle/checker_module_directives.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<module name="NewlineAtEndOfFile"/>
<!-- to enable @SuppressWarnings -->
<module name="SuppressWarningsFilter"/>
22 changes: 22 additions & 0 deletions config/checkstyle/checkstyle.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
"-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
"https://checkstyle.org/dtds/configuration_1_3.dtd" [
<!ENTITY checkerModuleDirectives SYSTEM "checker_module_directives.xml">
<!ENTITY treeWalkerModuleDirectives SYSTEM "tree_walker_module_directives.xml">
<!ENTITY prohibitJavaCollections SYSTEM "prohibit_java_collections.xml">
]>

<!-- Examples from this file: https://github.com/checkstyle/checkstyle/blob/master/src/main/resources/google_checks.xml -->

<!-- List of directives: https://checkstyle.sourceforge.io/version/4.4/availablechecks.html -->

<!-- DTD ENTITY-ies are used to allow decouple config file to smaller parts that can be reimported in submodule-specific configuration file -->

<module name="Checker">
&checkerModuleDirectives;

PawelLipski marked this conversation as resolved.
Show resolved Hide resolved
<module name="TreeWalker">
&treeWalkerModuleDirectives;
</module>
</module>
14 changes: 14 additions & 0 deletions config/checkstyle/import_control_prohibit_java_collections.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?xml version="1.0"?>
<!DOCTYPE import-control PUBLIC
"-//Checkstyle//DTD ImportControl Configuration 1.4//EN"
"https://checkstyle.org/dtds/import_control_1_4.dtd">

<import-control pkg="com.virtuslab" strategyOnMismatch="allowed" >
<disallow class="java.util.List" />
<disallow class="java.util.Map" />
<disallow class="java.util.Set" />
<disallow class="java.util.Queue" />
<disallow class="java.util.Deque" />
<disallow class="java.util.Vector" />
<disallow class="java.util.Stack" />
</import-control>
3 changes: 3 additions & 0 deletions config/checkstyle/prohibit_java_collections.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<module name="ImportControl">
<property name="file" value="${rootCheckstyleConfigDir}/import_control_prohibit_java_collections.xml"/>
</module>
66 changes: 66 additions & 0 deletions config/checkstyle/tree_walker_module_directives.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<!-- to enable @SuppressWarnings -->
<module name="SuppressWarningsHolder"/>

<module name="ArrayTypeStyle"/>
<module name="AvoidStarImport"/>
<module name="ConstantName">
<property name="format" value="^.*[Ii]nstance.*|.*[Ii]njector.*|([A-Z][A-Z0-9]*(_[A-Z0-9]+)*)$"/>
micpiotrowski marked this conversation as resolved.
Show resolved Hide resolved
</module>
<module name="CovariantEquals"/>
<module name="DefaultComesLast" />
<module name="EmptyBlock">
<property name="option" value="TEXT"/>
<property name="tokens"
value="LITERAL_TRY, LITERAL_FINALLY, LITERAL_IF, LITERAL_ELSE, LITERAL_SWITCH"/>
</module>
<module name="EmptyStatement"/>
<module name="EqualsHashCode"/>
<!-- <module name="ExplicitInitialization"/> -->
<module name="FallThrough"/>
<module name="FinalClass" />
<module name="HiddenField">
<property name="ignoreConstructorParameter" value="true"/>
</module>
<module name="HideUtilityClassConstructor" />
<module name="IllegalCatch"/>
<module name="IllegalThrows"/>
<module name="IllegalType"/>
<module name="InnerAssignment"/>
<module name="LeftCurly">
<property name="tokens"
value="ANNOTATION_DEF, CLASS_DEF, CTOR_DEF, ENUM_CONSTANT_DEF, ENUM_DEF,
INTERFACE_DEF, LAMBDA, LITERAL_CASE, LITERAL_CATCH, LITERAL_DEFAULT,
LITERAL_DO, LITERAL_ELSE, LITERAL_FINALLY, LITERAL_FOR, LITERAL_IF,
LITERAL_SWITCH, LITERAL_SYNCHRONIZED, LITERAL_TRY, LITERAL_WHILE, METHOD_DEF,
OBJBLOCK, STATIC_INIT"/>
</module>
<module name="MissingSwitchDefault"/>
<module name="MultipleStringLiterals">
<property name="allowedDuplicates" value="3"/>
</module>
<module name="MultipleVariableDeclarations"/>
<module name="ModifiedControlVariable"/>
<module name="ModifierOrder"/>
<module name="MutableException"/>
<module name="NeedBraces">
<property name="tokens"
value="LITERAL_DO, LITERAL_ELSE, LITERAL_FOR, LITERAL_IF, LITERAL_WHILE"/>
</module>
<module name="NestedTryDepth"/>
<module name="NoWhitespaceAfter"/>
<module name="NoWhitespaceBefore"/>
<module name="OneStatementPerLine"/>
<module name="PackageDeclaration"/>
<module name="ParameterAssignment"/>
<module name="RedundantModifier"/>
<module name="RightCurly">
<property name="id" value="RightCurlySame"/>
<property name="tokens"
value="LITERAL_TRY, LITERAL_CATCH, LITERAL_FINALLY, LITERAL_IF, LITERAL_ELSE,
LITERAL_DO"/>
</module>
<module name="StringLiteralEquality"/>
<module name="SuperClone"/>
<module name="UnnecessaryParentheses"/>
<module name="UnusedImports"/>
<module name="UpperEll"/>
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,23 @@
public final class ColorDefinitions {
private ColorDefinitions() {}

private static final Color red = Color.decode("#FF0000");
private static final Color orange = Color.decode("#FDA909");
private static final Color dark_orange = Color.decode("#D68C00");
private static final Color yellow = Color.decode("#C4A000");
private static final Color green = Color.decode("#008000");
private static final Color gray = Color.decode("#AAAAAA");
private static final Color dark_gray = Color.decode("#555555");
private static final Color transparent = new Color(0, 0, 0, 0);
private static final Color RED_COLOR = Color.decode("#FF0000");
private static final Color ORANGE_COLOR = Color.decode("#FDA909");
private static final Color DARK_ORANGE_COLOR = Color.decode("#D68C00");
private static final Color YELLOW_COLOR = Color.decode("#C4A000");
private static final Color GREEN_COLOR = Color.decode("#008000");
private static final Color GRAY_COLOR = Color.decode("#AAAAAA");
private static final Color DARK_COLOR = Color.decode("#555555");
private static final Color TRANSPARENT_COLOR = new Color(0, 0, 0, 0);

/**
* {@link com.intellij.ui.JBColor} are pairs of colors for light and dark IDE theme. LHS colors names are their
* "general" description. They may differ from RHS (which are actual and specific colors).
*/
public static final JBColor RED = new JBColor(red, red);
public static final JBColor ORANGE = new JBColor(dark_orange, orange);
public static final JBColor YELLOW = new JBColor(yellow, yellow);
public static final JBColor GREEN = new JBColor(green, green);
public static final JBColor GRAY = new JBColor(dark_gray, gray);
public static final JBColor TRANSPARENT = new JBColor(transparent, transparent);
public static final JBColor RED = new JBColor(RED_COLOR, RED_COLOR);
public static final JBColor ORANGE = new JBColor(DARK_ORANGE_COLOR, ORANGE_COLOR);
public static final JBColor YELLOW = new JBColor(YELLOW_COLOR, YELLOW_COLOR);
public static final JBColor GREEN = new JBColor(GREEN_COLOR, GREEN_COLOR);
public static final JBColor GRAY = new JBColor(DARK_COLOR, GRAY_COLOR);
public static final JBColor TRANSPARENT = new JBColor(TRANSPARENT_COLOR, TRANSPARENT_COLOR);
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ public final class GraphEdgeColorToJBColorMapper {

private GraphEdgeColorToJBColorMapper() {}

private static final Map<GraphEdgeColor, JBColor> colors = Map.of(
private static final Map<GraphEdgeColor, JBColor> COLORS = Map.of(
GraphEdgeColor.GRAY, ColorDefinitions.GRAY,
GraphEdgeColor.YELLOW, ColorDefinitions.YELLOW,
GraphEdgeColor.RED, ColorDefinitions.RED,
GraphEdgeColor.GREEN, ColorDefinitions.GREEN);

public static JBColor getColor(GraphEdgeColor graphEdgeColor) {
return colors.getOrDefault(graphEdgeColor, ColorDefinitions.TRANSPARENT);
return COLORS.getOrDefault(graphEdgeColor, ColorDefinitions.TRANSPARENT);
}

public static JBColor getColor(int colorId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@
public final class SyncToOriginStatusToTextColorMapper {
private SyncToOriginStatusToTextColorMapper() {}

private static final Map<Integer, JBColor> colors = Map.of(
private static final Map<Integer, JBColor> COLORS = Map.of(
Untracked.getId(), ORANGE,
Ahead.getId(), RED,
Behind.getId(), RED,
Diverged.getId(), RED);

public static JBColor getColor(int statusId) {
return colors.getOrDefault(statusId, JBColor.GRAY);
return COLORS.getOrDefault(statusId, JBColor.GRAY);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@
public final class SyncToParentStatusToGraphEdgeColorMapper {
private SyncToParentStatusToGraphEdgeColorMapper() {}

private static final Map<SyncToParentStatus, GraphEdgeColor> edges = Map.of(
private static final Map<SyncToParentStatus, GraphEdgeColor> EDGES = Map.of(
SyncToParentStatus.Merged, GraphEdgeColor.GRAY,
SyncToParentStatus.InSyncButForkPointOff, GraphEdgeColor.YELLOW,
SyncToParentStatus.OutOfSync, GraphEdgeColor.RED,
SyncToParentStatus.InSync, GraphEdgeColor.GREEN);

public static GraphEdgeColor getGraphEdgeColor(SyncToParentStatus syncToParentStatus) {
return edges.getOrDefault(syncToParentStatus, GraphEdgeColor.TRANSPARENT);
return EDGES.getOrDefault(syncToParentStatus, GraphEdgeColor.TRANSPARENT);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ public BranchElement(
this.attributes = isCurrentBranch ? UNDERLINE_BOLD_ATTRIBUTES : NORMAL_ATTRIBUTES;
}

private static final JBColor branchTextColor = new JBColor(Color.BLACK, Color.WHITE);
private static final JBColor BRANCH_TEXT_COLOR = new JBColor(Color.BLACK, Color.WHITE);

private static final SimpleTextAttributes UNDERLINE_BOLD_ATTRIBUTES = new SimpleTextAttributes(
SimpleTextAttributes.STYLE_UNDERLINE | SimpleTextAttributes.STYLE_BOLD, branchTextColor);
SimpleTextAttributes.STYLE_UNDERLINE | SimpleTextAttributes.STYLE_BOLD, BRANCH_TEXT_COLOR);

private static final SimpleTextAttributes NORMAL_ATTRIBUTES = new SimpleTextAttributes(
SimpleTextAttributes.STYLE_PLAIN, branchTextColor);
SimpleTextAttributes.STYLE_PLAIN, BRANCH_TEXT_COLOR);

@Getter
private final SimpleTextAttributes attributes;
Expand Down