From ee20ebf616358e64ea06e5af4b6d6db5b7d82155 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Mon, 22 Jan 2018 14:19:27 +0100 Subject: [PATCH 01/81] Add TestKotlinProject to setup a test kotlin project --- .../com/novoda/test/TestKotlinProject.groovy | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy diff --git a/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy new file mode 100644 index 0000000..28cf88b --- /dev/null +++ b/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy @@ -0,0 +1,37 @@ +package com.novoda.test + +final class TestKotlinProject extends TestProject { + + private static final Closure TEMPLATE = { TestProject project -> + """ +plugins { + id 'com.novoda.static-analysis' +} +repositories { + jcenter() +} +apply plugin: 'kotlin' +sourceSets { + ${formatSourceSets(project)} +} +${formatExtension(project)} +""" + } + + TestKotlinProject() { + super(TEMPLATE) + } + + private static String formatSourceSets(TestProject project) { + project.sourceSets + .entrySet() + .collect { Map.Entry> entry -> + """$entry.key { + kotlin { + ${entry.value.collect { "srcDir '$it'" }.join('\n\t\t\t\t')} + } + }""" + } + .join('\n\t') + } +} From 26b52d42341d9ccd1717487851490a7af472bb57 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Mon, 22 Jan 2018 15:06:40 +0100 Subject: [PATCH 02/81] Support multiple plugins for test projects --- .../com/novoda/test/TestAndroidProject.groovy | 2 +- .../groovy/com/novoda/test/TestJavaProject.groovy | 2 +- .../com/novoda/test/TestKotlinProject.groovy | 2 +- .../groovy/com/novoda/test/TestProject.groovy | 15 +++++++++++++++ 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/plugin/src/test/groovy/com/novoda/test/TestAndroidProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestAndroidProject.groovy index a879075..9de01e8 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestAndroidProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestAndroidProject.groovy @@ -13,7 +13,7 @@ buildscript { } } plugins { - id 'com.novoda.static-analysis' + ${formatPlugins(project)} } repositories { google() diff --git a/plugin/src/test/groovy/com/novoda/test/TestJavaProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestJavaProject.groovy index d1a6421..e014658 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestJavaProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestJavaProject.groovy @@ -5,7 +5,7 @@ final class TestJavaProject extends TestProject { private static final Closure TEMPLATE = { TestProject project -> """ plugins { - id 'com.novoda.static-analysis' + ${formatPlugins(project)} } repositories { jcenter() diff --git a/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy index 28cf88b..1e55e4b 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy @@ -5,7 +5,7 @@ final class TestKotlinProject extends TestProject { private static final Closure TEMPLATE = { TestProject project -> """ plugins { - id 'com.novoda.static-analysis' + ${formatPlugins(project)} } repositories { jcenter() diff --git a/plugin/src/test/groovy/com/novoda/test/TestProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestProject.groovy index f24247e..d99aa0a 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestProject.groovy @@ -22,6 +22,7 @@ ${project.additionalConfiguration} private final Closure template String additionalConfiguration = '' Map> sourceSets = [main: []] + List additionalPlugins = [] String penalty String toolsConfig = '' @@ -33,6 +34,7 @@ ${project.additionalConfiguration} .withPluginClasspath() .forwardStdOutput(new OutputStreamWriter(System.out)) .forwardStdError(new OutputStreamWriter(System.out)) + withPlugins('com.novoda.static-analysis') } private static File createProjectDir(String path) { @@ -82,6 +84,15 @@ ${project.additionalConfiguration} return this } + public T withPlugins(String... plugins) { + def formattedPlugins = plugins.collect { plugin -> + "id '$plugin'" + }.asList() + + additionalPlugins.addAll(formattedPlugins) + return this + } + public Result build(String... arguments) { BuildResult buildResult = newRunner(arguments).build() createResult(buildResult) @@ -113,6 +124,10 @@ ${project.additionalConfiguration} EXTENSION_TEMPLATE.call(project) } + protected static String formatPlugins(TestProject project) { + "${project.additionalPlugins.join(',\n')}" + } + public static class Result { private final BuildResult buildResult private final File buildDir From 1aa8d837baa5136b82d2014da52aa7ef5d63ec34 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Mon, 22 Jan 2018 16:04:28 +0100 Subject: [PATCH 03/81] Add detekt kotlin fixtures --- .../src/test/fixtures/rules/detekt/detekt.yml | 367 ++++++++++++++++++ .../sources/detekt/warnings/Warnings.kt | 9 + .../groovy/com/novoda/test/Fixtures.groovy | 5 + 3 files changed, 381 insertions(+) create mode 100644 plugin/src/test/fixtures/rules/detekt/detekt.yml create mode 100644 plugin/src/test/fixtures/sources/detekt/warnings/Warnings.kt diff --git a/plugin/src/test/fixtures/rules/detekt/detekt.yml b/plugin/src/test/fixtures/rules/detekt/detekt.yml new file mode 100644 index 0000000..6f3c5f6 --- /dev/null +++ b/plugin/src/test/fixtures/rules/detekt/detekt.yml @@ -0,0 +1,367 @@ +autoCorrect: true +failFast: false + +test-pattern: # Configure exclusions for test sources + active: true + patterns: # Test file regexes + - '.*/test/.*' + - '.*Test.kt' + - '.*Spec.kt' + exclude-rule-sets: + - 'comments' + exclude-rules: + - 'NamingRules' + - 'WildcardImport' + - 'MagicNumber' + - 'MaxLineLength' + - 'LateinitUsage' + - 'StringLiteralDuplication' + - 'SpreadOperator' + - 'TooManyFunctions' + +build: + warningThreshold: 5 + failThreshold: 10 + weights: + complexity: 2 + formatting: 1 + LongParameterList: 1 + comments: 1 + +processors: + active: true + exclude: + # - 'FunctionCountProcessor' + # - 'PropertyCountProcessor' + # - 'ClassCountProcessor' + # - 'PackageCountProcessor' + # - 'KtFileCountProcessor' + +console-reports: + active: true + exclude: + # - 'ProjectStatisticsReport' + # - 'ComplexityReport' + # - 'NotificationReport' + # - 'FindingsReport' + # - 'BuildFailureReport' + +output-reports: + active: true + exclude: + # - 'PlainOutputReport' + # - 'XmlOutputReport' + +comments: + active: true + CommentOverPrivateFunction: + active: true + CommentOverPrivateProperty: + active: false + UndocumentedPublicClass: + active: false + searchInNestedClass: true + searchInInnerClass: true + searchInInnerObject: true + searchInInnerInterface: true + UndocumentedPublicFunction: + active: false + +complexity: + active: true + ComplexCondition: + active: true + threshold: 3 + ComplexInterface: + active: false + threshold: 10 + includeStaticDeclarations: false + ComplexMethod: + active: true + threshold: 10 + LabeledExpression: + active: false + LargeClass: + active: true + threshold: 150 + LongMethod: + active: true + threshold: 20 + LongParameterList: + active: true + threshold: 5 + ignoreDefaultParameters: false + MethodOverloading: + active: false + threshold: 5 + NestedBlockDepth: + active: true + threshold: 3 + StringLiteralDuplication: + active: false + threshold: 2 + ignoreAnnotation: true + excludeStringsWithLessThan5Characters: true + ignoreStringsRegex: '$^' + TooManyFunctions: + active: true + thresholdInFiles: 10 + thresholdInClasses: 10 + thresholdInInterfaces: 10 + thresholdInObjects: 10 + thresholdInEnums: 10 + +empty-blocks: + active: true + EmptyCatchBlock: + active: true + EmptyClassBlock: + active: true + EmptyDefaultConstructor: + active: true + EmptyDoWhileBlock: + active: true + EmptyElseBlock: + active: true + EmptyFinallyBlock: + active: true + EmptyForBlock: + active: true + EmptyFunctionBlock: + active: true + EmptyIfBlock: + active: true + EmptyInitBlock: + active: true + EmptyKtFile: + active: true + EmptySecondaryConstructor: + active: true + EmptyWhenBlock: + active: true + EmptyWhileBlock: + active: true + +exceptions: + active: true + ExceptionRaisedInUnexpectedLocation: + active: false + methodNames: 'toString,hashCode,equals,finalize' + InstanceOfCheckForException: + active: false + NotImplementedDeclaration: + active: false + PrintStackTrace: + active: false + RethrowCaughtException: + active: false + ReturnFromFinally: + active: false + SwallowedException: + active: false + ThrowingExceptionFromFinally: + active: false + ThrowingExceptionInMain: + active: false + ThrowingExceptionsWithoutMessageOrCause: + active: false + exceptions: 'IllegalArgumentException,IllegalStateException,IOException' + ThrowingNewInstanceOfSameException: + active: false + TooGenericExceptionCaught: + active: true + exceptions: + - ArrayIndexOutOfBoundsException + - Error + - Exception + - IllegalMonitorStateException + - NullPointerException + - IndexOutOfBoundsException + - RuntimeException + - Throwable + TooGenericExceptionThrown: + active: true + exceptions: + - Error + - Exception + - NullPointerException + - Throwable + - RuntimeException + +naming: + active: true + ClassNaming: + active: true + classPattern: '[A-Z$][a-zA-Z0-9$]*' + EnumNaming: + active: true + enumEntryPattern: '^[A-Z$][a-zA-Z_$]*$' + ForbiddenClassName: + active: false + forbiddenName: '' + FunctionMaxLength: + active: false + maximumFunctionNameLength: 30 + FunctionMinLength: + active: false + minimumFunctionNameLength: 3 + FunctionNaming: + active: true + functionPattern: '^([a-z$][a-zA-Z$0-9]*)|(`.*`)$' + MatchingDeclarationName: + active: true + MemberNameEqualsClassName: + active: false + ignoreOverriddenFunction: true + ObjectPropertyNaming: + active: true + propertyPattern: '[A-Za-z][_A-Za-z0-9]*' + PackageNaming: + active: true + packagePattern: '^[a-z]+(\.[a-z][a-z0-9]*)*$' + TopLevelPropertyNaming: + active: true + constantPattern: '[A-Z][_A-Z0-9]*' + propertyPattern: '[a-z][A-Za-z\d]*' + privatePropertyPattern: '(_)?[a-z][A-Za-z0-9]*' + VariableMaxLength: + active: false + maximumVariableNameLength: 64 + VariableMinLength: + active: false + minimumVariableNameLength: 1 + VariableNaming: + active: true + variablePattern: '[a-z][A-Za-z0-9]*' + privateVariablePattern: '(_)?[a-z][A-Za-z0-9]*' + +performance: + active: true + ForEachOnRange: + active: true + SpreadOperator: + active: true + UnnecessaryTemporaryInstantiation: + active: true + +potential-bugs: + active: true + DuplicateCaseInWhenExpression: + active: true + EqualsAlwaysReturnsTrueOrFalse: + active: false + EqualsWithHashCodeExist: + active: true + ExplicitGarbageCollectionCall: + active: true + InvalidRange: + active: false + IteratorHasNextCallsNextMethod: + active: false + IteratorNotThrowingNoSuchElementException: + active: false + LateinitUsage: + active: false + excludeAnnotatedProperties: "" + ignoreOnClassesPattern: "" + UnconditionalJumpStatementInLoop: + active: false + UnreachableCode: + active: true + UnsafeCallOnNullableType: + active: false + UnsafeCast: + active: false + UselessPostfixExpression: + active: false + WrongEqualsTypeParameter: + active: false + +style: + active: true + CollapsibleIfStatements: + active: false + DataClassContainsFunctions: + active: false + conversionFunctionPrefix: 'to' + EqualsNullCall: + active: false + ExpressionBodySyntax: + active: false + ForbiddenComment: + active: true + values: 'TODO:,FIXME:,STOPSHIP:' + ForbiddenImport: + active: false + imports: '' + FunctionOnlyReturningConstant: + active: false + ignoreOverridableFunction: true + excludedFunctions: 'describeContents' + LoopWithTooManyJumpStatements: + active: false + maxJumpCount: 1 + MagicNumber: + active: true + ignoreNumbers: '-1,0,1,2' + ignoreHashCodeFunction: false + ignorePropertyDeclaration: false + ignoreConstantDeclaration: true + ignoreCompanionObjectPropertyDeclaration: true + ignoreAnnotation: false + ignoreNamedArgument: true + ignoreEnums: false + MaxLineLength: + active: true + maxLineLength: 120 + excludePackageStatements: false + excludeImportStatements: false + ModifierOrder: + active: true + NestedClassesVisibility: + active: false + NewLineAtEndOfFile: + active: true + OptionalAbstractKeyword: + active: true + OptionalReturnKeyword: + active: false + OptionalUnit: + active: false + OptionalWhenBraces: + active: false + ProtectedMemberInFinalClass: + active: false + RedundantVisibilityModifierRule: + active: false + ReturnCount: + active: true + max: 2 + excludedFunctions: "equals" + SafeCast: + active: true + SerialVersionUIDInSerializableClass: + active: false + SpacingBetweenPackageAndImports: + active: false + ThrowsCount: + active: true + max: 2 + UnnecessaryAbstractClass: + active: false + UnnecessaryInheritance: + active: false + UnnecessaryParentheses: + active: false + UntilInsteadOfRangeTo: + active: false + UnusedImports: + active: false + UseDataClass: + active: false + excludeAnnotatedClasses: "" + UtilityClassWithPublicConstructor: + active: false + WildcardImport: + active: true + excludeImports: 'java.util.*,kotlinx.android.synthetic.*' \ No newline at end of file diff --git a/plugin/src/test/fixtures/sources/detekt/warnings/Warnings.kt b/plugin/src/test/fixtures/sources/detekt/warnings/Warnings.kt new file mode 100644 index 0000000..16bf968 --- /dev/null +++ b/plugin/src/test/fixtures/sources/detekt/warnings/Warnings.kt @@ -0,0 +1,9 @@ +class Warnings { + + private fun foo() { + for (i in 1..2) { + break + println() // unreachable + } + } +} \ No newline at end of file diff --git a/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy b/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy index 4ae624d..ad0b1ab 100644 --- a/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy +++ b/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy @@ -44,4 +44,9 @@ public final class Fixtures { public static final File SAMPLE_REPORT = new File(REPORTS_DIR, 'findbugs/reports/sample.xml') } + final static class Detekt{ + public static final File SOURCES_WITH_WARNINGS = new File(SOURCES_DIR, 'detekt/warnings') + public static final File RULES = new File(RULES_DIR, 'detekt/detekt.yml') + } + } From 6bb5a1fdbb15dd45359dbbdb2853dcc49f058e9d Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Mon, 22 Jan 2018 16:05:47 +0100 Subject: [PATCH 04/81] Expose kotlin test project rule --- plugin/src/test/groovy/com/novoda/test/TestProjectRule.groovy | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/plugin/src/test/groovy/com/novoda/test/TestProjectRule.groovy b/plugin/src/test/groovy/com/novoda/test/TestProjectRule.groovy index ebd2f8c..930a3ba 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestProjectRule.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestProjectRule.groovy @@ -19,6 +19,10 @@ final class TestProjectRule implements TestRule { new TestProjectRule({ new TestAndroidProject() }, { String name -> "project.android.sourceSets.$name" }, 'Android project') } + static TestProjectRule forKotlinProject() { + new TestProjectRule({ new TestKotlinProject() }, { String name -> "project.sourceSets.$name" }, 'Kotlin project') + } + private TestProjectRule(Closure projectFactory, Closure sourceSetNameFactory, String label) { this.projectFactory = projectFactory this.sourceSetNameFactory = sourceSetNameFactory From 9d7b9d1c441cf55ea271476a9779f6116054c418 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Mon, 22 Jan 2018 16:06:27 +0100 Subject: [PATCH 05/81] Add kotlin support to TestKotlinProject --- .../com/novoda/test/TestKotlinProject.groovy | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy index 1e55e4b..786e250 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy @@ -4,12 +4,20 @@ final class TestKotlinProject extends TestProject { private static final Closure TEMPLATE = { TestProject project -> """ +buildscript { + repositories { + google() + jcenter() + } + dependencies { + classpath 'org.jetbrains.kotlin:kotlin-gradle-plugin:1.1.4-3' + } +} + plugins { ${formatPlugins(project)} } -repositories { - jcenter() -} + apply plugin: 'kotlin' sourceSets { ${formatSourceSets(project)} From 3c03ba47da5801de83e4531cf5248e2b13e924cd Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Mon, 22 Jan 2018 16:07:33 +0100 Subject: [PATCH 06/81] Expose projectDir to configure an output dir for detekt used in the tests --- plugin/src/test/groovy/com/novoda/test/TestProject.groovy | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/plugin/src/test/groovy/com/novoda/test/TestProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestProject.groovy index d99aa0a..f794963 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestProject.groovy @@ -120,6 +120,10 @@ ${project.additionalConfiguration} projectDir.deleteDir() } + String projectDir(){ + return projectDir + } + protected static String formatExtension(TestProject project) { EXTENSION_TEMPLATE.call(project) } From a87451e34cd43aa615018efc12d7cac62348ce79 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Mon, 22 Jan 2018 16:08:27 +0100 Subject: [PATCH 07/81] Add DetektIntegrationTest which verifies the build fails when detekt warnings exceed threshold --- .../detekt/DetektIntegrationTest.groovy | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy new file mode 100644 index 0000000..e99ded6 --- /dev/null +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy @@ -0,0 +1,63 @@ +package com.novoda.staticanalysis.internal.detekt + +import com.novoda.test.DeployRulesTestRule +import com.novoda.test.Fixtures +import com.novoda.test.TestProject +import com.novoda.test.TestProjectRule +import org.junit.ClassRule +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized + +import static com.novoda.test.Fixtures.Findbugs.SOURCES_WITH_HIGH_VIOLATION +import static com.novoda.test.LogsSubject.assertThat + +@RunWith(Parameterized.class) +class DetektIntegrationTest { + + @ClassRule + public static final DeployRulesTestRule DEPLOY_RULES = new DeployRulesTestRule( + resourceDirs: [Fixtures.RULES_DIR], + repoDir: new File(Fixtures.BUILD_DIR, 'rules')) + + @Parameterized.Parameters(name = "{0}") + static Iterable rules() { + return [TestProjectRule.forKotlinProject()] + } + + @Rule + public final TestProjectRule projectRule + + DetektIntegrationTest(TestProjectRule projectRule) { + this.projectRule = projectRule + } + + @Test + void shouldFailBuildWhenDetektWarningsOverTheThreshold() { + def testProject = projectRule.newProject() + .withSourceSet('main', Fixtures.Detekt.SOURCES_WITH_WARNINGS) + .withPenalty('''{ + maxWarnings = 0 + maxErrors = 0 + }''') + + def detektConfiguration = """ + detekt { + profile('main') { + config = "${Fixtures.Detekt.RULES}" + output = "${testProject.projectDir()}/build/reports" + } + } + """ + + testProject = testProject.withToolsConfig(detektConfiguration) + + TestProject.Result result = testProject + .buildAndFail('check') + + assertThat(result.logs).containsLimitExceeded(0, 1) + } + +} + From 54f982b66905b1f58ec9358f4aa3a1286aebbe3b Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Mon, 22 Jan 2018 16:48:59 +0100 Subject: [PATCH 08/81] Simplify Warning Fixture --- .../internal/detekt/DetektConfigurator.groovy | 107 ++++++++++++++++++ .../sources/detekt/warnings/Warnings.kt | 2 +- 2 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy new file mode 100644 index 0000000..0becb70 --- /dev/null +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -0,0 +1,107 @@ +package com.novoda.staticanalysis.internal + +import com.novoda.staticanalysis.StaticAnalysisExtension +import org.gradle.api.Action +import org.gradle.api.NamedDomainObjectSet +import org.gradle.api.Project +import org.gradle.api.Task +import org.gradle.api.plugins.quality.CodeQualityExtension +import org.gradle.api.tasks.SourceTask + +abstract class CodeQualityConfigurator { + + protected final Project project + protected final Violations violations + protected final Task evaluateViolations + protected final SourceFilter sourceFilter + protected Closure includeVariantsFilter + + protected CodeQualityConfigurator(Project project, Violations violations, Task evaluateViolations) { + this.project = project + this.violations = violations + this.evaluateViolations = evaluateViolations + this.sourceFilter = new SourceFilter(project) + this.includeVariantsFilter = { true } + } + + void execute() { + project.extensions.findByType(StaticAnalysisExtension).ext."$toolName" = { Closure config -> + project.apply plugin: toolPlugin + project.extensions.findByType(extensionClass).with { + defaultConfiguration.execute(it) + ext.exclude = { Object rule -> sourceFilter.exclude(rule) } + ext.includeVariants = { Closure filter -> includeVariantsFilter = filter } + config.delegate = it + config() + } + project.plugins.withId('com.android.application') { + project.afterEvaluate { + configureAndroidProject(allApplicationVariants.matching { includeVariantsFilter(it) }) + configureToolTasks() + } + } + project.plugins.withId('com.android.library') { + project.afterEvaluate { + configureAndroidProject(allLibraryVariants.matching { includeVariantsFilter(it) }) + configureToolTasks() + } + } + project.plugins.withId('java') { + project.afterEvaluate { + configureJavaProject() + configureToolTasks() + } + } + } + } + + protected NamedDomainObjectSet getAllApplicationVariants() { + getAllVariants(project.android.applicationVariants) + } + + protected void configureToolTasks() { + project.tasks.withType(taskClass) { task -> + task.group = 'verification' + configureReportEvaluation(task, violations) + } + } + + protected NamedDomainObjectSet getAllLibraryVariants() { + getAllVariants(project.android.libraryVariants) + } + + private NamedDomainObjectSet getAllVariants(variants1) { + NamedDomainObjectSet variants = project.container(Object) + variants.addAll(variants1) + variants.addAll(project.android.testVariants) + variants.addAll(project.android.unitTestVariants) + return variants + } + + protected abstract String getToolName() + + protected Object getToolPlugin() { + toolName + } + + protected abstract Class getExtensionClass() + + protected Action getDefaultConfiguration() { + new Action() { + void execute(E ignored) { + // no op + } + } + } + + protected abstract void configureAndroidProject(NamedDomainObjectSet variants) + + protected void configureJavaProject() { + project.tasks.withType(taskClass) { task -> sourceFilter.applyTo(task) } + } + + protected abstract Class getTaskClass() + + protected abstract void configureReportEvaluation(T task, Violations violations) + +} diff --git a/plugin/src/test/fixtures/sources/detekt/warnings/Warnings.kt b/plugin/src/test/fixtures/sources/detekt/warnings/Warnings.kt index 16bf968..f640ab3 100644 --- a/plugin/src/test/fixtures/sources/detekt/warnings/Warnings.kt +++ b/plugin/src/test/fixtures/sources/detekt/warnings/Warnings.kt @@ -3,7 +3,7 @@ class Warnings { private fun foo() { for (i in 1..2) { break - println() // unreachable + val number = 1 // unreachable } } } \ No newline at end of file From e1d1512e2b0ca6224059e2c32745177c4e074a19 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Mon, 22 Jan 2018 16:56:38 +0100 Subject: [PATCH 09/81] Add skeleton for DetektConfigurator --- .../StaticAnalysisPlugin.groovy | 4 + .../internal/detekt/DetektConfigurator.groovy | 104 +++--------------- 2 files changed, 21 insertions(+), 87 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy index b853eff..277a14f 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy @@ -3,6 +3,7 @@ package com.novoda.staticanalysis import com.novoda.staticanalysis.internal.CodeQualityConfigurator import com.novoda.staticanalysis.internal.Violations import com.novoda.staticanalysis.internal.checkstyle.CheckstyleConfigurator +import com.novoda.staticanalysis.internal.detekt.DetektConfigurator import com.novoda.staticanalysis.internal.findbugs.FindbugsConfigurator import com.novoda.staticanalysis.internal.pmd.PmdConfigurator import org.gradle.api.NamedDomainObjectContainer @@ -17,6 +18,9 @@ class StaticAnalysisPlugin implements Plugin { StaticAnalysisExtension pluginExtension = project.extensions.create('staticAnalysis', StaticAnalysisExtension, project) Task evaluateViolations = createEvaluateViolationsTask(project, pluginExtension) createConfigurators(project, pluginExtension, evaluateViolations).each { configurator -> configurator.execute() } + + new DetektConfigurator(project).execute() + project.afterEvaluate { project.tasks['check'].dependsOn evaluateViolations } diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index 0becb70..10860bf 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -1,107 +1,37 @@ -package com.novoda.staticanalysis.internal +package com.novoda.staticanalysis.internal.detekt import com.novoda.staticanalysis.StaticAnalysisExtension -import org.gradle.api.Action -import org.gradle.api.NamedDomainObjectSet import org.gradle.api.Project -import org.gradle.api.Task -import org.gradle.api.plugins.quality.CodeQualityExtension -import org.gradle.api.tasks.SourceTask -abstract class CodeQualityConfigurator { +public class DetektConfigurator { protected final Project project - protected final Violations violations - protected final Task evaluateViolations - protected final SourceFilter sourceFilter - protected Closure includeVariantsFilter - protected CodeQualityConfigurator(Project project, Violations violations, Task evaluateViolations) { + protected DetektConfigurator(Project project) { this.project = project - this.violations = violations - this.evaluateViolations = evaluateViolations - this.sourceFilter = new SourceFilter(project) - this.includeVariantsFilter = { true } } void execute() { - project.extensions.findByType(StaticAnalysisExtension).ext."$toolName" = { Closure config -> - project.apply plugin: toolPlugin - project.extensions.findByType(extensionClass).with { - defaultConfiguration.execute(it) - ext.exclude = { Object rule -> sourceFilter.exclude(rule) } - ext.includeVariants = { Closure filter -> includeVariantsFilter = filter } - config.delegate = it - config() - } - project.plugins.withId('com.android.application') { - project.afterEvaluate { - configureAndroidProject(allApplicationVariants.matching { includeVariantsFilter(it) }) - configureToolTasks() - } - } - project.plugins.withId('com.android.library') { - project.afterEvaluate { - configureAndroidProject(allLibraryVariants.matching { includeVariantsFilter(it) }) - configureToolTasks() - } - } - project.plugins.withId('java') { - project.afterEvaluate { - configureJavaProject() - configureToolTasks() - } - } - } - } - - protected NamedDomainObjectSet getAllApplicationVariants() { - getAllVariants(project.android.applicationVariants) - } - - protected void configureToolTasks() { - project.tasks.withType(taskClass) { task -> - task.group = 'verification' - configureReportEvaluation(task, violations) - } - } + project.extensions.findByType(StaticAnalysisExtension).ext."detekt" = { Closure config -> - protected NamedDomainObjectSet getAllLibraryVariants() { - getAllVariants(project.android.libraryVariants) - } - - private NamedDomainObjectSet getAllVariants(variants1) { - NamedDomainObjectSet variants = project.container(Object) - variants.addAll(variants1) - variants.addAll(project.android.testVariants) - variants.addAll(project.android.unitTestVariants) - return variants - } - - protected abstract String getToolName() - - protected Object getToolPlugin() { - toolName - } + if (!isKotlinProject(project)) { + return + } - protected abstract Class getExtensionClass() + if (project.tasks.findByName('detektCheck')) { + def detektTask = project.tasks.findByName('detektCheck') + project.tasks.findByName('check').dependsOn(detektTask) - protected Action getDefaultConfiguration() { - new Action() { - void execute(E ignored) { - // no op } } } - protected abstract void configureAndroidProject(NamedDomainObjectSet variants) - - protected void configureJavaProject() { - project.tasks.withType(taskClass) { task -> sourceFilter.applyTo(task) } + private static boolean isKotlinProject(final Project project) { + final boolean isKotlin = project.plugins.hasPlugin('kotlin') + final boolean isKotlinAndroid = project.plugins.hasPlugin('kotlin-android') + final boolean isKotlinPlatformCommon = project.plugins.hasPlugin('kotlin-platform-common') + final boolean isKotlinPlatformJvm = project.plugins.hasPlugin('kotlin-platform-jvm') + final boolean isKotlinPlatformJs = project.plugins.hasPlugin('kotlin-platform-js') + return isKotlin || isKotlinAndroid || isKotlinPlatformCommon || isKotlinPlatformJvm || isKotlinPlatformJs } - - protected abstract Class getTaskClass() - - protected abstract void configureReportEvaluation(T task, Violations violations) - } From 44c2b082a74e6e780ec46d5d4fe6ae44e43c33cd Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Mon, 22 Jan 2018 17:12:55 +0100 Subject: [PATCH 10/81] Add detekt to test project classpath --- .../test/groovy/com/novoda/test/TestKotlinProject.groovy | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy index 786e250..d8f3f37 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy @@ -6,11 +6,14 @@ final class TestKotlinProject extends TestProject { """ buildscript { repositories { - google() jcenter() + maven { + url "https://plugins.gradle.org/m2/" + } } dependencies { - classpath 'org.jetbrains.kotlin:kotlin-gradle-plugin:1.1.4-3' + classpath 'org.jetbrains.kotlin:kotlin-gradle-plugin:1.2.10' + classpath 'gradle.plugin.io.gitlab.arturbosch.detekt:detekt-gradle-plugin:1.0.0.RC6-2' } } From 913a6e7d9dd74ce2422a3f03c9bd19fbfe00628f Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Mon, 22 Jan 2018 17:13:22 +0100 Subject: [PATCH 11/81] Apply detekt plugin in case the project has kotlin configured --- .../staticanalysis/internal/detekt/DetektConfigurator.groovy | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index 10860bf..b22c95a 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -18,6 +18,8 @@ public class DetektConfigurator { return } + project.apply plugin: 'io.gitlab.arturbosch.detekt' + if (project.tasks.findByName('detektCheck')) { def detektTask = project.tasks.findByName('detektCheck') project.tasks.findByName('check').dependsOn(detektTask) From 5b775fece1eca17a855d577bc0ebf3595bc421af Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Mon, 22 Jan 2018 17:59:14 +0100 Subject: [PATCH 12/81] Apply detekt configuration to the detekt extension --- .../internal/detekt/DetektConfigurator.groovy | 6 ++++++ .../internal/detekt/DetektIntegrationTest.groovy | 1 + 2 files changed, 7 insertions(+) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index b22c95a..768d590 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -20,6 +20,12 @@ public class DetektConfigurator { project.apply plugin: 'io.gitlab.arturbosch.detekt' + project.extensions.findByName('detekt').with { + // apply configuration closure to detekt extension + config.delegate = it + config() + } + if (project.tasks.findByName('detektCheck')) { def detektTask = project.tasks.findByName('detektCheck') project.tasks.findByName('check').dependsOn(detektTask) diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy index e99ded6..848aa3d 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy @@ -47,6 +47,7 @@ class DetektIntegrationTest { profile('main') { config = "${Fixtures.Detekt.RULES}" output = "${testProject.projectDir()}/build/reports" + input = "${Fixtures.Detekt.SOURCES_WITH_WARNINGS}" } } """ From 432bc1f07d02d460ead7de8414580d41a5c770e8 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 23 Jan 2018 10:50:47 +0100 Subject: [PATCH 13/81] Fix broken Fixture path --- .../detekt/CollectDetektViolationsTask.groovy | 17 +++++++++++++++++ .../test/groovy/com/novoda/test/Fixtures.groovy | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/CollectDetektViolationsTask.groovy diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/CollectDetektViolationsTask.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/CollectDetektViolationsTask.groovy new file mode 100644 index 0000000..90beb29 --- /dev/null +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/CollectDetektViolationsTask.groovy @@ -0,0 +1,17 @@ +package com.novoda.staticanalysis.internal.checkstyle + +import com.novoda.staticanalysis.internal.CollectViolationsTask +import com.novoda.staticanalysis.internal.Violations +import groovy.util.slurpersupport.GPathResult + +class CollectCheckstyleViolationsTask extends CollectViolationsTask { + + @Override + void collectViolations(File xmlReportFile, File htmlReportFile, Violations violations) { + GPathResult xml = new XmlSlurper().parse(xmlReportFile) + int errors = xml.'**'.findAll { node -> node.name() == 'error' && node.@severity == 'error' }.size() + int warnings = xml.'**'.findAll { node -> node.name() == 'error' && node.@severity == 'warning' }.size() + violations.addViolations(errors, warnings, htmlReportFile ?: xmlReportFile) + } + +} diff --git a/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy b/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy index ad0b1ab..ef1f474 100644 --- a/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy +++ b/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy @@ -5,7 +5,7 @@ import com.google.common.io.Resources public final class Fixtures { private static final Exception NO_INSTANCE_ALLOWED = new UnsupportedOperationException("No instance allowed"); private static final File ROOT_DIR = new File(Resources.getResource('.').file).parentFile.parentFile.parentFile.parentFile - private static final File FIXTURES_DIR = new File(ROOT_DIR, 'src/test/fixtures') + private static final File FIXTURES_DIR = new File(ROOT_DIR, 'plugin/src/test/fixtures') private static final File SOURCES_DIR = new File(FIXTURES_DIR, 'sources') private static final File REPORTS_DIR = new File(FIXTURES_DIR, 'reports') public static final File RULES_DIR = new File(FIXTURES_DIR, 'rules') From b00ed49da2894f6f9a94f59e03ff2c2961b18dac Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 23 Jan 2018 10:57:27 +0100 Subject: [PATCH 14/81] Add empty line to fixture file in order to suppress not wanted detekt style warning --- plugin/src/test/fixtures/sources/detekt/warnings/Warnings.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/test/fixtures/sources/detekt/warnings/Warnings.kt b/plugin/src/test/fixtures/sources/detekt/warnings/Warnings.kt index f640ab3..5f3ac1d 100644 --- a/plugin/src/test/fixtures/sources/detekt/warnings/Warnings.kt +++ b/plugin/src/test/fixtures/sources/detekt/warnings/Warnings.kt @@ -6,4 +6,4 @@ class Warnings { val number = 1 // unreachable } } -} \ No newline at end of file +} From 390308999dcba1cbed6ae31482d0fc9336a66fce Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 23 Jan 2018 11:13:35 +0100 Subject: [PATCH 15/81] Add CollectDetektViolationsTask --- .../internal/detekt/CollectDetektViolationsTask.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/CollectDetektViolationsTask.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/CollectDetektViolationsTask.groovy index 90beb29..326007f 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/CollectDetektViolationsTask.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/CollectDetektViolationsTask.groovy @@ -1,10 +1,10 @@ -package com.novoda.staticanalysis.internal.checkstyle +package com.novoda.staticanalysis.internal.detekt import com.novoda.staticanalysis.internal.CollectViolationsTask import com.novoda.staticanalysis.internal.Violations import groovy.util.slurpersupport.GPathResult -class CollectCheckstyleViolationsTask extends CollectViolationsTask { +class CollectDetektViolationsTask extends CollectViolationsTask { @Override void collectViolations(File xmlReportFile, File htmlReportFile, Violations violations) { From fdd3756225f29b634794b9f2db24d57420f97170 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 23 Jan 2018 11:16:19 +0100 Subject: [PATCH 16/81] Configure detekt task to collect violations --- .../StaticAnalysisPlugin.groovy | 2 +- .../internal/detekt/DetektConfigurator.groovy | 35 ++++++++++++++++--- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy index 277a14f..d062158 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy @@ -19,7 +19,7 @@ class StaticAnalysisPlugin implements Plugin { Task evaluateViolations = createEvaluateViolationsTask(project, pluginExtension) createConfigurators(project, pluginExtension, evaluateViolations).each { configurator -> configurator.execute() } - new DetektConfigurator(project).execute() + new DetektConfigurator(project, pluginExtension.allViolations.maybeCreate("Detekt"), evaluateViolations).execute() project.afterEvaluate { project.tasks['check'].dependsOn evaluateViolations diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index 768d590..f9dbf6c 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -1,14 +1,20 @@ package com.novoda.staticanalysis.internal.detekt import com.novoda.staticanalysis.StaticAnalysisExtension +import com.novoda.staticanalysis.internal.Violations import org.gradle.api.Project +import org.gradle.api.Task public class DetektConfigurator { - protected final Project project + private final Project project + private final Violations violations + private final Task evaluateViolations - protected DetektConfigurator(Project project) { + public DetektConfigurator(Project project, Violations violations, Task evaluateViolations) { this.project = project + this.violations = violations + this.evaluateViolations = evaluateViolations } void execute() { @@ -20,6 +26,7 @@ public class DetektConfigurator { project.apply plugin: 'io.gitlab.arturbosch.detekt' + project.extensions.findByName('detekt').with { // apply configuration closure to detekt extension config.delegate = it @@ -27,13 +34,31 @@ public class DetektConfigurator { } if (project.tasks.findByName('detektCheck')) { - def detektTask = project.tasks.findByName('detektCheck') - project.tasks.findByName('check').dependsOn(detektTask) - + configureToolTask() } } } + private void configureToolTask() { + def detektTask = project.tasks.findByName('detektCheck') + // run detekt as part of check + project.tasks.findByName('check').dependsOn(detektTask) + + // evaluate violations after detekt + detektTask.group = 'verification' + def collectViolations = createCollectViolationsTask(violations) + evaluateViolations.dependsOn collectViolations + collectViolations.dependsOn detektTask + } + + private CollectDetektViolationsTask createCollectViolationsTask(Violations violations) { + project.tasks.create("collectDetektViolations", CollectDetektViolationsTask) { collectViolations -> + //TODO: resolve path to report file + collectViolations.xmlReportFile = null + collectViolations.violations = violations + } + } + private static boolean isKotlinProject(final Project project) { final boolean isKotlin = project.plugins.hasPlugin('kotlin') final boolean isKotlinAndroid = project.plugins.hasPlugin('kotlin-android') From f5c85cae387b88c61a15a0393ef000c28a1d2ef7 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 23 Jan 2018 11:28:17 +0100 Subject: [PATCH 17/81] Revert Fixture path change --- plugin/src/test/groovy/com/novoda/test/Fixtures.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy b/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy index ef1f474..ad0b1ab 100644 --- a/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy +++ b/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy @@ -5,7 +5,7 @@ import com.google.common.io.Resources public final class Fixtures { private static final Exception NO_INSTANCE_ALLOWED = new UnsupportedOperationException("No instance allowed"); private static final File ROOT_DIR = new File(Resources.getResource('.').file).parentFile.parentFile.parentFile.parentFile - private static final File FIXTURES_DIR = new File(ROOT_DIR, 'plugin/src/test/fixtures') + private static final File FIXTURES_DIR = new File(ROOT_DIR, 'src/test/fixtures') private static final File SOURCES_DIR = new File(FIXTURES_DIR, 'sources') private static final File REPORTS_DIR = new File(FIXTURES_DIR, 'reports') public static final File RULES_DIR = new File(FIXTURES_DIR, 'rules') From c61e8e2b62ba5670c3a7a7d6f4167b6cc271ae47 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 23 Jan 2018 12:05:33 +0100 Subject: [PATCH 18/81] Link detekt report file to CollectDetektViolationsTask --- .../internal/detekt/DetektConfigurator.groovy | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index f9dbf6c..29db4e8 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -26,7 +26,6 @@ public class DetektConfigurator { project.apply plugin: 'io.gitlab.arturbosch.detekt' - project.extensions.findByName('detekt').with { // apply configuration closure to detekt extension config.delegate = it @@ -52,9 +51,10 @@ public class DetektConfigurator { } private CollectDetektViolationsTask createCollectViolationsTask(Violations violations) { + def reportFilePath = "${project.extensions.findByName('detekt').systemOrDefaultProfile().output}/detekt-checkstyle.xml" + project.tasks.create("collectDetektViolations", CollectDetektViolationsTask) { collectViolations -> - //TODO: resolve path to report file - collectViolations.xmlReportFile = null + collectViolations.xmlReportFile = new File(reportFilePath) collectViolations.violations = violations } } From c9af099df8586cb9ab551d841f3c6b893f5f41ec Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 23 Jan 2018 12:08:30 +0100 Subject: [PATCH 19/81] Remove not needed public keyword --- .../staticanalysis/internal/detekt/DetektConfigurator.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index 29db4e8..b76bd60 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -5,13 +5,13 @@ import com.novoda.staticanalysis.internal.Violations import org.gradle.api.Project import org.gradle.api.Task -public class DetektConfigurator { +class DetektConfigurator { private final Project project private final Violations violations private final Task evaluateViolations - public DetektConfigurator(Project project, Violations violations, Task evaluateViolations) { + DetektConfigurator(Project project, Violations violations, Task evaluateViolations) { this.project = project this.violations = violations this.evaluateViolations = evaluateViolations From eea5b69aa651dbd056ab081f91f32a95cb6206ab Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 23 Jan 2018 12:20:31 +0100 Subject: [PATCH 20/81] Extract common Configurator interface --- .../staticanalysis/internal/CodeQualityConfigurator.groovy | 3 ++- .../com/novoda/staticanalysis/internal/Configurator.groovy | 6 ++++++ .../internal/detekt/DetektConfigurator.groovy | 4 +++- 3 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 plugin/src/main/groovy/com/novoda/staticanalysis/internal/Configurator.groovy diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/CodeQualityConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/CodeQualityConfigurator.groovy index 0becb70..e8fe42f 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/CodeQualityConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/CodeQualityConfigurator.groovy @@ -8,7 +8,7 @@ import org.gradle.api.Task import org.gradle.api.plugins.quality.CodeQualityExtension import org.gradle.api.tasks.SourceTask -abstract class CodeQualityConfigurator { +abstract class CodeQualityConfigurator implements Configurator { protected final Project project protected final Violations violations @@ -24,6 +24,7 @@ abstract class CodeQualityConfigurator project.apply plugin: toolPlugin diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/Configurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/Configurator.groovy new file mode 100644 index 0000000..1f32766 --- /dev/null +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/Configurator.groovy @@ -0,0 +1,6 @@ +package com.novoda.staticanalysis.internal + +interface Configurator { + + void execute() +} diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index b76bd60..b383d7c 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -1,11 +1,12 @@ package com.novoda.staticanalysis.internal.detekt import com.novoda.staticanalysis.StaticAnalysisExtension +import com.novoda.staticanalysis.internal.Configurator import com.novoda.staticanalysis.internal.Violations import org.gradle.api.Project import org.gradle.api.Task -class DetektConfigurator { +class DetektConfigurator implements Configurator{ private final Project project private final Violations violations @@ -17,6 +18,7 @@ class DetektConfigurator { this.evaluateViolations = evaluateViolations } + @Override void execute() { project.extensions.findByType(StaticAnalysisExtension).ext."detekt" = { Closure config -> From 0bc714dc271a249787e3c3fcd9984e9471949f19 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 23 Jan 2018 12:23:32 +0100 Subject: [PATCH 21/81] Create DetektConfigurator as part of other Configurators --- .../staticanalysis/StaticAnalysisPlugin.groovy | 5 ++--- .../internal/detekt/DetektConfigurator.groovy | 12 ++++++++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy index d062158..8c8595e 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy @@ -19,8 +19,6 @@ class StaticAnalysisPlugin implements Plugin { Task evaluateViolations = createEvaluateViolationsTask(project, pluginExtension) createConfigurators(project, pluginExtension, evaluateViolations).each { configurator -> configurator.execute() } - new DetektConfigurator(project, pluginExtension.allViolations.maybeCreate("Detekt"), evaluateViolations).execute() - project.afterEvaluate { project.tasks['check'].dependsOn evaluateViolations } @@ -41,7 +39,8 @@ class StaticAnalysisPlugin implements Plugin { [ CheckstyleConfigurator.create(project, violationsContainer, evaluateViolations), PmdConfigurator.create(project, violationsContainer, evaluateViolations), - FindbugsConfigurator.create(project, violationsContainer, evaluateViolations) + FindbugsConfigurator.create(project, violationsContainer, evaluateViolations), + DetektConfigurator.create(project, violationsContainer, evaluateViolations) ] } } diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index b383d7c..ab3d5a1 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -3,16 +3,24 @@ package com.novoda.staticanalysis.internal.detekt import com.novoda.staticanalysis.StaticAnalysisExtension import com.novoda.staticanalysis.internal.Configurator import com.novoda.staticanalysis.internal.Violations +import org.gradle.api.NamedDomainObjectContainer import org.gradle.api.Project import org.gradle.api.Task -class DetektConfigurator implements Configurator{ +class DetektConfigurator implements Configurator { private final Project project private final Violations violations private final Task evaluateViolations - DetektConfigurator(Project project, Violations violations, Task evaluateViolations) { + static DetektConfigurator create(Project project, + NamedDomainObjectContainer violationsContainer, + Task evaluateViolations) { + Violations violations = violationsContainer.maybeCreate('Detekt') + return new DetektConfigurator(project, violations, evaluateViolations) + } + + private DetektConfigurator(Project project, Violations violations, Task evaluateViolations) { this.project = project this.violations = violations this.evaluateViolations = evaluateViolations From 9b4cf95175b0f0848a01ea149552c1b24f16a674 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 23 Jan 2018 12:36:13 +0100 Subject: [PATCH 22/81] Add a test case to verify the the build fails when detekt errors are over threshold --- .../fixtures/sources/detekt/errors/Errors.kt | 7 +++++ .../detekt/DetektIntegrationTest.groovy | 27 +++++++++++++++++++ .../groovy/com/novoda/test/Fixtures.groovy | 1 + 3 files changed, 35 insertions(+) create mode 100644 plugin/src/test/fixtures/sources/detekt/errors/Errors.kt diff --git a/plugin/src/test/fixtures/sources/detekt/errors/Errors.kt b/plugin/src/test/fixtures/sources/detekt/errors/Errors.kt new file mode 100644 index 0000000..8125c1e --- /dev/null +++ b/plugin/src/test/fixtures/sources/detekt/errors/Errors.kt @@ -0,0 +1,7 @@ +class Warnings { + + override fun equals(other: Any?): Boolean { + // this is not allowed + return true + } +} diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy index 848aa3d..def1dba 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy @@ -60,5 +60,32 @@ class DetektIntegrationTest { assertThat(result.logs).containsLimitExceeded(0, 1) } + @Test + void shouldFailBuildWhenDetektErrorsOverTheThreshold() { + def testProject = projectRule.newProject() + .withSourceSet('main', Fixtures.Detekt.SOURCES_WITH_WARNINGS) + .withPenalty('''{ + maxWarnings = 0 + maxErrors = 0 + }''') + + def detektConfiguration = """ + detekt { + profile('main') { + config = "${Fixtures.Detekt.RULES}" + output = "${testProject.projectDir()}/build/reports" + input = "${Fixtures.Detekt.SOURCES_WITH_ERRORS}" + } + } + """ + + testProject = testProject.withToolsConfig(detektConfiguration) + + TestProject.Result result = testProject + .buildAndFail('check') + + assertThat(result.logs).containsLimitExceeded(1, 0) + } + } diff --git a/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy b/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy index ad0b1ab..bf04607 100644 --- a/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy +++ b/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy @@ -46,6 +46,7 @@ public final class Fixtures { final static class Detekt{ public static final File SOURCES_WITH_WARNINGS = new File(SOURCES_DIR, 'detekt/warnings') + public static final File SOURCES_WITH_ERRORS = new File(SOURCES_DIR, 'detekt/errors') public static final File RULES = new File(RULES_DIR, 'detekt/detekt.yml') } From 1b2a1541bf958fb4ee62aefc647a9d3b7df39c52 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 23 Jan 2018 12:47:49 +0100 Subject: [PATCH 23/81] Verify the build does not fail in case detekt is not configured or thresholds are not exceeded --- .../detekt/DetektIntegrationTest.groovy | 74 +++++++++++++++++++ .../groovy/com/novoda/test/LogsSubject.groovy | 9 +++ 2 files changed, 83 insertions(+) diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy index def1dba..cb81f16 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy @@ -58,6 +58,8 @@ class DetektIntegrationTest { .buildAndFail('check') assertThat(result.logs).containsLimitExceeded(0, 1) + assertThat(result.logs).containsDetektViolations(0, 1, + result.buildFileUrl('reports/detekt-checkstyle.html')) } @Test @@ -85,7 +87,79 @@ class DetektIntegrationTest { .buildAndFail('check') assertThat(result.logs).containsLimitExceeded(1, 0) + assertThat(result.logs).containsDetektViolations(1, 0, + result.buildFileUrl('reports/detekt-checkstyle.html')) } + @Test + void shouldNotFailWhenDetektIsNotConfigured() throws Exception { + def testProject = projectRule.newProject() + .withSourceSet('main', Fixtures.Detekt.SOURCES_WITH_WARNINGS) + .withPenalty('''{ + maxWarnings = 0 + maxErrors = 0 + }''') + .build('check') + + TestProject.Result result = testProject + + assertThat(result.logs).doesNotContainDetektViolations() + } + + @Test + void shouldNotFailWhenWarningsAreBelowThreshold() throws Exception { + def testProject = projectRule.newProject() + .withSourceSet('main', Fixtures.Detekt.SOURCES_WITH_WARNINGS) + .withPenalty('''{ + maxWarnings = 1 + maxErrors = 0 + }''') + + def detektConfiguration = """ + detekt { + profile('main') { + config = "${Fixtures.Detekt.RULES}" + output = "${testProject.projectDir()}/build/reports" + input = "${Fixtures.Detekt.SOURCES_WITH_WARNINGS}" + } + } + """ + + testProject = testProject.withToolsConfig(detektConfiguration) + + TestProject.Result result = testProject + .build('check') + + assertThat(result.logs).containsDetektViolations(0, 1, + result.buildFileUrl('reports/detekt-checkstyle.html')) + } + + @Test + void shouldNotFailWhenErrorsAreBelowThreshold() throws Exception { + def testProject = projectRule.newProject() + .withSourceSet('main', Fixtures.Detekt.SOURCES_WITH_WARNINGS) + .withPenalty('''{ + maxWarnings = 0 + maxErrors = 1 + }''') + + def detektConfiguration = """ + detekt { + profile('main') { + config = "${Fixtures.Detekt.RULES}" + output = "${testProject.projectDir()}/build/reports" + input = "${Fixtures.Detekt.SOURCES_WITH_ERRORS}" + } + } + """ + + testProject = testProject.withToolsConfig(detektConfiguration) + + TestProject.Result result = testProject + .build('check') + + assertThat(result.logs).containsDetektViolations(1, 0, + result.buildFileUrl('reports/detekt-checkstyle.html')) + } } diff --git a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy index 6eb037c..58b6e83 100644 --- a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy @@ -16,6 +16,7 @@ class LogsSubject extends Subject { private static final String CHECKSTYLE_VIOLATIONS_FOUND = "Checkstyle violations found" private static final String PMD_VIOLATIONS_FOUND = "PMD violations found" private static final String FINDBUGS_VIOLATIONS_FOUND = "Findbugs violations found" + private static final String DETEKT_VIOLATIONS_FOUND = "Detekt violations found" private static final SubjectFactory FACTORY = new SubjectFactory() { @Override LogsSubject getSubject(FailureStrategy failureStrategy, Logs logs) { @@ -59,6 +60,10 @@ class LogsSubject extends Subject { outputSubject.doesNotContain(FINDBUGS_VIOLATIONS_FOUND) } + public void doesNotContainDetektViolations() { + outputSubject.doesNotContain(DETEKT_VIOLATIONS_FOUND) + } + public void containsCheckstyleViolations(int errors, int warnings, String... reportUrls) { containsToolViolations(CHECKSTYLE_VIOLATIONS_FOUND, errors, warnings, reportUrls) } @@ -71,6 +76,10 @@ class LogsSubject extends Subject { containsToolViolations(FINDBUGS_VIOLATIONS_FOUND, errors, warnings, reportUrls) } + public void containsDetektViolations(int errors, int warnings, String... reportUrls) { + containsToolViolations(DETEKT_VIOLATIONS_FOUND, errors, warnings, reportUrls) + } + private void containsToolViolations(String template, int errors, int warnings, String... reportUrls) { outputSubject.contains("$template ($errors errors, $warnings warnings). See the reports at:\n") for (String reportUrl : reportUrls) { From 5b5277165cd83c705574bee7740db9ded29abd10 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 23 Jan 2018 12:55:02 +0100 Subject: [PATCH 24/81] Should not fail when no warnings or errors threshold trespassed --- .../detekt/DetektIntegrationTest.groovy | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy index cb81f16..c778423 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy @@ -161,5 +161,31 @@ class DetektIntegrationTest { assertThat(result.logs).containsDetektViolations(1, 0, result.buildFileUrl('reports/detekt-checkstyle.html')) } + + @Test + void shouldNotFailBuildWhenNoDetektWarningsOrErrorsEncounteredAndNoThresholdTrespassed() { + def testProject = projectRule.newProject() + .withPenalty('''{ + maxWarnings = 0 + maxErrors = 0 + }''') + + def detektConfiguration = """ + detekt { + profile('main') { + config = "${Fixtures.Detekt.RULES}" + output = "${testProject.projectDir()}/build/reports" + } + } + """ + + testProject = testProject.withToolsConfig(detektConfiguration) + + TestProject.Result result = testProject + .build('check') + + assertThat(result.logs).doesNotContainLimitExceeded() + assertThat(result.logs).doesNotContainDetektViolations() + } } From 924969062045214b988335a0591891b25487220f Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 23 Jan 2018 15:26:47 +0100 Subject: [PATCH 25/81] Remove empty line --- .../groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy | 1 - 1 file changed, 1 deletion(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy index 8c8595e..13582f8 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/StaticAnalysisPlugin.groovy @@ -18,7 +18,6 @@ class StaticAnalysisPlugin implements Plugin { StaticAnalysisExtension pluginExtension = project.extensions.create('staticAnalysis', StaticAnalysisExtension, project) Task evaluateViolations = createEvaluateViolationsTask(project, pluginExtension) createConfigurators(project, pluginExtension, evaluateViolations).each { configurator -> configurator.execute() } - project.afterEvaluate { project.tasks['check'].dependsOn evaluateViolations } From faee71d44cdaf5868885110e77e8c083df71ec25 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 23 Jan 2018 15:31:54 +0100 Subject: [PATCH 26/81] Rename field --- plugin/src/test/groovy/com/novoda/test/TestProject.groovy | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugin/src/test/groovy/com/novoda/test/TestProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestProject.groovy index f794963..dbd6a7b 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestProject.groovy @@ -22,7 +22,7 @@ ${project.additionalConfiguration} private final Closure template String additionalConfiguration = '' Map> sourceSets = [main: []] - List additionalPlugins = [] + List plugins = [] String penalty String toolsConfig = '' @@ -89,7 +89,7 @@ ${project.additionalConfiguration} "id '$plugin'" }.asList() - additionalPlugins.addAll(formattedPlugins) + this.plugins.addAll(formattedPlugins) return this } @@ -129,7 +129,7 @@ ${project.additionalConfiguration} } protected static String formatPlugins(TestProject project) { - "${project.additionalPlugins.join(',\n')}" + "${project.plugins.join(',\n')}" } public static class Result { From 7a825125e01ed34de3edc638358cc4f885225f2d Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 23 Jan 2018 15:35:57 +0100 Subject: [PATCH 27/81] Rename class --- plugin/src/test/fixtures/sources/detekt/errors/Errors.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/test/fixtures/sources/detekt/errors/Errors.kt b/plugin/src/test/fixtures/sources/detekt/errors/Errors.kt index 8125c1e..a92d86e 100644 --- a/plugin/src/test/fixtures/sources/detekt/errors/Errors.kt +++ b/plugin/src/test/fixtures/sources/detekt/errors/Errors.kt @@ -1,4 +1,4 @@ -class Warnings { +class Errors { override fun equals(other: Any?): Boolean { // this is not allowed From 403006101a46004d357b41b6a1c93330fbad3ce7 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 23 Jan 2018 16:36:29 +0100 Subject: [PATCH 28/81] Simplify task access --- .../staticanalysis/internal/detekt/DetektConfigurator.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index ab3d5a1..c9d45c2 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -49,9 +49,9 @@ class DetektConfigurator implements Configurator { } private void configureToolTask() { - def detektTask = project.tasks.findByName('detektCheck') + def detektTask = project.tasks['detektCheck'] // run detekt as part of check - project.tasks.findByName('check').dependsOn(detektTask) + project.tasks['check'].dependsOn(detektTask) // evaluate violations after detekt detektTask.group = 'verification' From 64405bb3535f05f16c4446d6d7478af2a56bb575 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 23 Jan 2018 17:37:52 +0100 Subject: [PATCH 29/81] Fix wrong generic type --- plugin/src/test/groovy/com/novoda/test/TestProjectRule.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/test/groovy/com/novoda/test/TestProjectRule.groovy b/plugin/src/test/groovy/com/novoda/test/TestProjectRule.groovy index 930a3ba..ec32aeb 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestProjectRule.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestProjectRule.groovy @@ -19,7 +19,7 @@ final class TestProjectRule implements TestRule { new TestProjectRule({ new TestAndroidProject() }, { String name -> "project.android.sourceSets.$name" }, 'Android project') } - static TestProjectRule forKotlinProject() { + static TestProjectRule forKotlinProject() { new TestProjectRule({ new TestKotlinProject() }, { String name -> "project.sourceSets.$name" }, 'Kotlin project') } From 70b7b2fd44cea27a9191826c855f17d75e3472d7 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Tue, 23 Jan 2018 17:48:44 +0100 Subject: [PATCH 30/81] Add comment which might be useful for other developers --- .../internal/detekt/DetektIntegrationTest.groovy | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy index c778423..8f69413 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy @@ -47,6 +47,9 @@ class DetektIntegrationTest { profile('main') { config = "${Fixtures.Detekt.RULES}" output = "${testProject.projectDir()}/build/reports" + // The input just needs to be configured for the tests. + // Probably detekt doesn't pick up the changed source sets. + // In a example project it was not needed. input = "${Fixtures.Detekt.SOURCES_WITH_WARNINGS}" } } From fd34d6bd516b2db3764fd9fcf4b3bc808503d5b8 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Wed, 24 Jan 2018 11:37:27 +0100 Subject: [PATCH 31/81] Improve test names --- .../internal/detekt/DetektIntegrationTest.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy index 8f69413..8ed757e 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy @@ -110,7 +110,7 @@ class DetektIntegrationTest { } @Test - void shouldNotFailWhenWarningsAreBelowThreshold() throws Exception { + void shouldNotFailWhenWarningsAreWithinThreshold() throws Exception { def testProject = projectRule.newProject() .withSourceSet('main', Fixtures.Detekt.SOURCES_WITH_WARNINGS) .withPenalty('''{ @@ -138,7 +138,7 @@ class DetektIntegrationTest { } @Test - void shouldNotFailWhenErrorsAreBelowThreshold() throws Exception { + void shouldNotFailWhenErrorsAreWithinThreshold() throws Exception { def testProject = projectRule.newProject() .withSourceSet('main', Fixtures.Detekt.SOURCES_WITH_WARNINGS) .withPenalty('''{ From 584c716eda3fb602c3d06557b64d83c5c9c4779e Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Wed, 24 Jan 2018 11:37:44 +0100 Subject: [PATCH 32/81] Remove extra white line --- .../staticanalysis/internal/detekt/DetektIntegrationTest.groovy | 1 - 1 file changed, 1 deletion(-) diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy index 8ed757e..21b108b 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy @@ -191,4 +191,3 @@ class DetektIntegrationTest { assertThat(result.logs).doesNotContainDetektViolations() } } - From 794b74c0c6c84708b23f054983a83a4107a362f5 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Wed, 24 Jan 2018 11:39:41 +0100 Subject: [PATCH 33/81] Fix format --- plugin/src/test/groovy/com/novoda/test/Fixtures.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy b/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy index bf04607..18cff92 100644 --- a/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy +++ b/plugin/src/test/groovy/com/novoda/test/Fixtures.groovy @@ -44,7 +44,7 @@ public final class Fixtures { public static final File SAMPLE_REPORT = new File(REPORTS_DIR, 'findbugs/reports/sample.xml') } - final static class Detekt{ + final static class Detekt { public static final File SOURCES_WITH_WARNINGS = new File(SOURCES_DIR, 'detekt/warnings') public static final File SOURCES_WITH_ERRORS = new File(SOURCES_DIR, 'detekt/errors') public static final File RULES = new File(RULES_DIR, 'detekt/detekt.yml') From c0f6ebaeedf71ecec73ee02e374ea7fb699bf578 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Thu, 25 Jan 2018 11:37:37 +0100 Subject: [PATCH 34/81] Support versions for adding a plugin --- .../groovy/com/novoda/test/TestProject.groovy | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/plugin/src/test/groovy/com/novoda/test/TestProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestProject.groovy index dbd6a7b..2b5ea20 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestProject.groovy @@ -34,7 +34,7 @@ ${project.additionalConfiguration} .withPluginClasspath() .forwardStdOutput(new OutputStreamWriter(System.out)) .forwardStdError(new OutputStreamWriter(System.out)) - withPlugins('com.novoda.static-analysis') + withPlugin('com.novoda.static-analysis', null) } private static File createProjectDir(String path) { @@ -84,15 +84,19 @@ ${project.additionalConfiguration} return this } - public T withPlugins(String... plugins) { - def formattedPlugins = plugins.collect { plugin -> - "id '$plugin'" - }.asList() - - this.plugins.addAll(formattedPlugins) + public T withPlugin(String plugin, String version) { + this.plugins.add("id \"$plugin\" ${optionalVersionFrom(version)}") return this } + private static String optionalVersionFrom(String version) { + if (version == null) { + return "" + } + "version \"$version\"" + } + + public Result build(String... arguments) { BuildResult buildResult = newRunner(arguments).build() createResult(buildResult) @@ -120,7 +124,7 @@ ${project.additionalConfiguration} projectDir.deleteDir() } - String projectDir(){ + String projectDir() { return projectDir } @@ -129,7 +133,7 @@ ${project.additionalConfiguration} } protected static String formatPlugins(TestProject project) { - "${project.plugins.join(',\n')}" + "${project.plugins.join('\n')}" } public static class Result { From c573067130539ad8a7f2ae2987888b90635f208b Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Thu, 25 Jan 2018 11:38:16 +0100 Subject: [PATCH 35/81] Do not apply detekt for each kotlin test project --- .../src/test/groovy/com/novoda/test/TestKotlinProject.groovy | 4 ---- 1 file changed, 4 deletions(-) diff --git a/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy index d8f3f37..89f80c0 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestKotlinProject.groovy @@ -7,13 +7,9 @@ final class TestKotlinProject extends TestProject { buildscript { repositories { jcenter() - maven { - url "https://plugins.gradle.org/m2/" - } } dependencies { classpath 'org.jetbrains.kotlin:kotlin-gradle-plugin:1.2.10' - classpath 'gradle.plugin.io.gitlab.arturbosch.detekt:detekt-gradle-plugin:1.0.0.RC6-2' } } From 0402b1a37111e14baa87801f97e9ed0bab4c7ead Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Thu, 25 Jan 2018 11:40:54 +0100 Subject: [PATCH 36/81] Apply detekt plugin manually in tests --- .../internal/detekt/DetektIntegrationTest.groovy | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy index 21b108b..36f8f60 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy @@ -10,7 +10,6 @@ import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.Parameterized -import static com.novoda.test.Fixtures.Findbugs.SOURCES_WITH_HIGH_VIOLATION import static com.novoda.test.LogsSubject.assertThat @RunWith(Parameterized.class) @@ -36,6 +35,7 @@ class DetektIntegrationTest { @Test void shouldFailBuildWhenDetektWarningsOverTheThreshold() { def testProject = projectRule.newProject() + .withPlugin("io.gitlab.arturbosch.detekt", "1.0.0.RC6-2") .withSourceSet('main', Fixtures.Detekt.SOURCES_WITH_WARNINGS) .withPenalty('''{ maxWarnings = 0 @@ -68,6 +68,7 @@ class DetektIntegrationTest { @Test void shouldFailBuildWhenDetektErrorsOverTheThreshold() { def testProject = projectRule.newProject() + .withPlugin("io.gitlab.arturbosch.detekt", "1.0.0.RC6-2") .withSourceSet('main', Fixtures.Detekt.SOURCES_WITH_WARNINGS) .withPenalty('''{ maxWarnings = 0 @@ -97,6 +98,7 @@ class DetektIntegrationTest { @Test void shouldNotFailWhenDetektIsNotConfigured() throws Exception { def testProject = projectRule.newProject() + .withPlugin("io.gitlab.arturbosch.detekt", "1.0.0.RC6-2") .withSourceSet('main', Fixtures.Detekt.SOURCES_WITH_WARNINGS) .withPenalty('''{ maxWarnings = 0 @@ -112,6 +114,7 @@ class DetektIntegrationTest { @Test void shouldNotFailWhenWarningsAreWithinThreshold() throws Exception { def testProject = projectRule.newProject() + .withPlugin("io.gitlab.arturbosch.detekt", "1.0.0.RC6-2") .withSourceSet('main', Fixtures.Detekt.SOURCES_WITH_WARNINGS) .withPenalty('''{ maxWarnings = 1 @@ -140,6 +143,7 @@ class DetektIntegrationTest { @Test void shouldNotFailWhenErrorsAreWithinThreshold() throws Exception { def testProject = projectRule.newProject() + .withPlugin("io.gitlab.arturbosch.detekt", "1.0.0.RC6-2") .withSourceSet('main', Fixtures.Detekt.SOURCES_WITH_WARNINGS) .withPenalty('''{ maxWarnings = 0 @@ -168,6 +172,7 @@ class DetektIntegrationTest { @Test void shouldNotFailBuildWhenNoDetektWarningsOrErrorsEncounteredAndNoThresholdTrespassed() { def testProject = projectRule.newProject() + .withPlugin("io.gitlab.arturbosch.detekt", "1.0.0.RC6-2") .withPenalty('''{ maxWarnings = 0 maxErrors = 0 From c27b3af70659b7e152d34c98cc68bc9b8dcf88d5 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Thu, 25 Jan 2018 11:42:16 +0100 Subject: [PATCH 37/81] Do not apply detekt as part of our plugin since we expect the user to do it manually --- .../staticanalysis/internal/detekt/DetektConfigurator.groovy | 2 -- 1 file changed, 2 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index c9d45c2..cf8e492 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -34,8 +34,6 @@ class DetektConfigurator implements Configurator { return } - project.apply plugin: 'io.gitlab.arturbosch.detekt' - project.extensions.findByName('detekt').with { // apply configuration closure to detekt extension config.delegate = it From 1b6b9c6c6b2ffc07a5c1611652ee396171c6b956 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Thu, 25 Jan 2018 11:48:35 +0100 Subject: [PATCH 38/81] Add a test which verifies we inform the user to manually apply detekt in case it's configured but not allplied --- .../detekt/DetektIntegrationTest.groovy | 25 +++++++++++++++++++ .../groovy/com/novoda/test/LogsSubject.groovy | 5 ++++ 2 files changed, 30 insertions(+) diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy index 36f8f60..8cb0910 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy @@ -195,4 +195,29 @@ class DetektIntegrationTest { assertThat(result.logs).doesNotContainLimitExceeded() assertThat(result.logs).doesNotContainDetektViolations() } + + @Test + void shouldFailBuildWhenNoDetektConfiguredButNotApplied() { + def testProject = projectRule.newProject() + .withPenalty('''{ + maxWarnings = 0 + maxErrors = 0 + }''') + + def detektConfiguration = """ + detekt { + profile('main') { + config = "${Fixtures.Detekt.RULES}" + output = "${testProject.projectDir()}/build/reports" + } + } + """ + + testProject = testProject.withToolsConfig(detektConfiguration) + + TestProject.Result result = testProject + .buildAndFail('check') + + assertThat(result.logs).containsDetektNotApplied() + } } diff --git a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy index 58b6e83..5e9b0cd 100644 --- a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy @@ -13,6 +13,7 @@ import static com.novoda.test.TestProject.Result.Logs; class LogsSubject extends Subject { private static final String VIOLATIONS_LIMIT_EXCEEDED = "Violations limit exceeded" + private static final String DETEKT_NOT_APPLIED = "The Detekt plugin is configured but not applied. Please apply the plugin in your build script." private static final String CHECKSTYLE_VIOLATIONS_FOUND = "Checkstyle violations found" private static final String PMD_VIOLATIONS_FOUND = "PMD violations found" private static final String FINDBUGS_VIOLATIONS_FOUND = "Findbugs violations found" @@ -40,6 +41,10 @@ class LogsSubject extends Subject { check().that(actual().output) } + public void containsDetektNotApplied() { + outputSubject.contains(DETEKT_NOT_APPLIED) + } + public void doesNotContainLimitExceeded() { outputSubject.doesNotContain(VIOLATIONS_LIMIT_EXCEEDED) } From b2e0a092943c5970b11f35d06b6b8e2c88de45d8 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Thu, 25 Jan 2018 11:51:52 +0100 Subject: [PATCH 39/81] Throw an exception in case the detekt plugin is configured but not applied --- .../internal/detekt/DetektConfigurator.groovy | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index cf8e492..8e6cd71 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -3,6 +3,7 @@ package com.novoda.staticanalysis.internal.detekt import com.novoda.staticanalysis.StaticAnalysisExtension import com.novoda.staticanalysis.internal.Configurator import com.novoda.staticanalysis.internal.Violations +import org.gradle.api.GradleException import org.gradle.api.NamedDomainObjectContainer import org.gradle.api.Project import org.gradle.api.Task @@ -34,15 +35,17 @@ class DetektConfigurator implements Configurator { return } + if (!project.tasks.findByName('detektCheck')) { + throw new GradleException('The Detekt plugin is configured but not applied. Please apply the plugin in your build script.') + } + project.extensions.findByName('detekt').with { // apply configuration closure to detekt extension config.delegate = it config() } - if (project.tasks.findByName('detektCheck')) { - configureToolTask() - } + configureToolTask() } } From 81a87eb40b15f887499f93ca244a7bb98520003a Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Thu, 25 Jan 2018 12:01:33 +0100 Subject: [PATCH 40/81] Fix typo --- .../staticanalysis/internal/detekt/DetektConfigurator.groovy | 2 +- .../staticanalysis/internal/detekt/DetektIntegrationTest.groovy | 2 +- plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index 8e6cd71..d3602aa 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -36,7 +36,7 @@ class DetektConfigurator implements Configurator { } if (!project.tasks.findByName('detektCheck')) { - throw new GradleException('The Detekt plugin is configured but not applied. Please apply the plugin in your build script.') + throw new GradleException('The detekt plugin is configured but not applied. Please apply the plugin in your build script.') } project.extensions.findByName('detekt').with { diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy index 8cb0910..c9bb01c 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy @@ -197,7 +197,7 @@ class DetektIntegrationTest { } @Test - void shouldFailBuildWhenNoDetektConfiguredButNotApplied() { + void shouldFailBuildWhenDetektConfiguredButNotApplied() { def testProject = projectRule.newProject() .withPenalty('''{ maxWarnings = 0 diff --git a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy index 5e9b0cd..cdeb87a 100644 --- a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy @@ -13,7 +13,7 @@ import static com.novoda.test.TestProject.Result.Logs; class LogsSubject extends Subject { private static final String VIOLATIONS_LIMIT_EXCEEDED = "Violations limit exceeded" - private static final String DETEKT_NOT_APPLIED = "The Detekt plugin is configured but not applied. Please apply the plugin in your build script." + private static final String DETEKT_NOT_APPLIED = "The detekt plugin is configured but not applied. Please apply the plugin in your build script." private static final String CHECKSTYLE_VIOLATIONS_FOUND = "Checkstyle violations found" private static final String PMD_VIOLATIONS_FOUND = "PMD violations found" private static final String FINDBUGS_VIOLATIONS_FOUND = "Findbugs violations found" From d9b1812680882db99134a53efc41bd7e0e8fbc6b Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Thu, 25 Jan 2018 12:53:42 +0100 Subject: [PATCH 41/81] Use default parameter instead of passing null --- plugin/src/test/groovy/com/novoda/test/TestProject.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/src/test/groovy/com/novoda/test/TestProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestProject.groovy index 2b5ea20..d1f6eee 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestProject.groovy @@ -34,7 +34,7 @@ ${project.additionalConfiguration} .withPluginClasspath() .forwardStdOutput(new OutputStreamWriter(System.out)) .forwardStdError(new OutputStreamWriter(System.out)) - withPlugin('com.novoda.static-analysis', null) + withPlugin('com.novoda.static-analysis') } private static File createProjectDir(String path) { @@ -84,7 +84,7 @@ ${project.additionalConfiguration} return this } - public T withPlugin(String plugin, String version) { + public T withPlugin(String plugin, String version = null) { this.plugins.add("id \"$plugin\" ${optionalVersionFrom(version)}") return this } From d45e5a3c263e2329ece073e11daa2c73b2b95b5e Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Thu, 25 Jan 2018 12:56:08 +0100 Subject: [PATCH 42/81] Check for the plugin rather than for a task --- .../internal/detekt/DetektConfigurator.groovy | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index d3602aa..66593b5 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -10,6 +10,9 @@ import org.gradle.api.Task class DetektConfigurator implements Configurator { + private static final String DETEKT_PLUGIN = 'io.gitlab.arturbosch.detekt' + private static final String DETEKT_NOT_APPLIED = 'The detekt plugin is configured but not applied. Please apply the plugin in your build script.' + private final Project project private final Violations violations private final Task evaluateViolations @@ -35,8 +38,8 @@ class DetektConfigurator implements Configurator { return } - if (!project.tasks.findByName('detektCheck')) { - throw new GradleException('The detekt plugin is configured but not applied. Please apply the plugin in your build script.') + if (!project.plugins.hasPlugin(DETEKT_PLUGIN)) { + throw new GradleException(DETEKT_NOT_APPLIED) } project.extensions.findByName('detekt').with { From 8ba85d1ceae771d4a4b42a4b1f2c8e052f0d3626 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Thu, 25 Jan 2018 15:47:02 +0100 Subject: [PATCH 43/81] Inline method --- .../src/test/groovy/com/novoda/test/TestProject.groovy | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/plugin/src/test/groovy/com/novoda/test/TestProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestProject.groovy index d1f6eee..0b53755 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestProject.groovy @@ -85,17 +85,10 @@ ${project.additionalConfiguration} } public T withPlugin(String plugin, String version = null) { - this.plugins.add("id \"$plugin\" ${optionalVersionFrom(version)}") + this.plugins.add("id '$plugin' ${version ? "version '$version'" : ""}") return this } - private static String optionalVersionFrom(String version) { - if (version == null) { - return "" - } - "version \"$version\"" - } - public Result build(String... arguments) { BuildResult buildResult = newRunner(arguments).build() From 2ecbc1ab20481e2d3c3516d1c5d7e67c2af67b76 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Thu, 25 Jan 2018 16:00:55 +0100 Subject: [PATCH 44/81] Extract detektConfiguration to improve test readability --- .../detekt/DetektIntegrationTest.groovy | 94 ++++++------------- 1 file changed, 29 insertions(+), 65 deletions(-) diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy index c9bb01c..6d69fa7 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy @@ -42,20 +42,7 @@ class DetektIntegrationTest { maxErrors = 0 }''') - def detektConfiguration = """ - detekt { - profile('main') { - config = "${Fixtures.Detekt.RULES}" - output = "${testProject.projectDir()}/build/reports" - // The input just needs to be configured for the tests. - // Probably detekt doesn't pick up the changed source sets. - // In a example project it was not needed. - input = "${Fixtures.Detekt.SOURCES_WITH_WARNINGS}" - } - } - """ - - testProject = testProject.withToolsConfig(detektConfiguration) + testProject = testProject.withToolsConfig(detektConfiguration(testProject, Fixtures.Detekt.SOURCES_WITH_WARNINGS)) TestProject.Result result = testProject .buildAndFail('check') @@ -75,17 +62,7 @@ class DetektIntegrationTest { maxErrors = 0 }''') - def detektConfiguration = """ - detekt { - profile('main') { - config = "${Fixtures.Detekt.RULES}" - output = "${testProject.projectDir()}/build/reports" - input = "${Fixtures.Detekt.SOURCES_WITH_ERRORS}" - } - } - """ - - testProject = testProject.withToolsConfig(detektConfiguration) + testProject = testProject.withToolsConfig(detektConfiguration(testProject, Fixtures.Detekt.SOURCES_WITH_ERRORS)) TestProject.Result result = testProject .buildAndFail('check') @@ -121,17 +98,7 @@ class DetektIntegrationTest { maxErrors = 0 }''') - def detektConfiguration = """ - detekt { - profile('main') { - config = "${Fixtures.Detekt.RULES}" - output = "${testProject.projectDir()}/build/reports" - input = "${Fixtures.Detekt.SOURCES_WITH_WARNINGS}" - } - } - """ - - testProject = testProject.withToolsConfig(detektConfiguration) + testProject = testProject.withToolsConfig(detektConfiguration(testProject, Fixtures.Detekt.SOURCES_WITH_WARNINGS)) TestProject.Result result = testProject .build('check') @@ -150,17 +117,7 @@ class DetektIntegrationTest { maxErrors = 1 }''') - def detektConfiguration = """ - detekt { - profile('main') { - config = "${Fixtures.Detekt.RULES}" - output = "${testProject.projectDir()}/build/reports" - input = "${Fixtures.Detekt.SOURCES_WITH_ERRORS}" - } - } - """ - - testProject = testProject.withToolsConfig(detektConfiguration) + testProject = testProject.withToolsConfig(detektConfiguration(testProject, Fixtures.Detekt.SOURCES_WITH_ERRORS)) TestProject.Result result = testProject .build('check') @@ -177,17 +134,7 @@ class DetektIntegrationTest { maxWarnings = 0 maxErrors = 0 }''') - - def detektConfiguration = """ - detekt { - profile('main') { - config = "${Fixtures.Detekt.RULES}" - output = "${testProject.projectDir()}/build/reports" - } - } - """ - - testProject = testProject.withToolsConfig(detektConfiguration) + testProject = testProject.withToolsConfig(detektConfigurationWithoutInput(testProject)) TestProject.Result result = testProject .build('check') @@ -204,20 +151,37 @@ class DetektIntegrationTest { maxErrors = 0 }''') - def detektConfiguration = """ + testProject = testProject.withToolsConfig(detektConfiguration(testProject, Fixtures.Detekt.SOURCES_WITH_ERRORS)) + + TestProject.Result result = testProject + .buildAndFail('check') + + assertThat(result.logs).containsDetektNotApplied() + } + + private static GString detektConfiguration(TestProject testProject, File input) { + """ detekt { profile('main') { config = "${Fixtures.Detekt.RULES}" output = "${testProject.projectDir()}/build/reports" + // The input just needs to be configured for the tests. + // Probably detekt doesn't pick up the changed source sets. + // In a example project it was not needed. + input = "${input}" } } """ + } - testProject = testProject.withToolsConfig(detektConfiguration) - - TestProject.Result result = testProject - .buildAndFail('check') - - assertThat(result.logs).containsDetektNotApplied() + private static GString detektConfigurationWithoutInput(TestProject testProject) { + """ + detekt { + profile('main') { + config = "${Fixtures.Detekt.RULES}" + output = "${testProject.projectDir()}/build/reports" + } + } + """ } } From 8cd6c1ba44c1779a3ce4d075ce5e8700e288a43a Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Thu, 25 Jan 2018 16:17:45 +0100 Subject: [PATCH 45/81] Extract configuration to improve readability --- .../detekt/DetektIntegrationTest.groovy | 84 +++++++------------ 1 file changed, 31 insertions(+), 53 deletions(-) diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy index 6d69fa7..a497dec 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy @@ -34,17 +34,7 @@ class DetektIntegrationTest { @Test void shouldFailBuildWhenDetektWarningsOverTheThreshold() { - def testProject = projectRule.newProject() - .withPlugin("io.gitlab.arturbosch.detekt", "1.0.0.RC6-2") - .withSourceSet('main', Fixtures.Detekt.SOURCES_WITH_WARNINGS) - .withPenalty('''{ - maxWarnings = 0 - maxErrors = 0 - }''') - - testProject = testProject.withToolsConfig(detektConfiguration(testProject, Fixtures.Detekt.SOURCES_WITH_WARNINGS)) - - TestProject.Result result = testProject + def result = createProjectWithZeroThreshold(Fixtures.Detekt.SOURCES_WITH_WARNINGS) .buildAndFail('check') assertThat(result.logs).containsLimitExceeded(0, 1) @@ -54,17 +44,7 @@ class DetektIntegrationTest { @Test void shouldFailBuildWhenDetektErrorsOverTheThreshold() { - def testProject = projectRule.newProject() - .withPlugin("io.gitlab.arturbosch.detekt", "1.0.0.RC6-2") - .withSourceSet('main', Fixtures.Detekt.SOURCES_WITH_WARNINGS) - .withPenalty('''{ - maxWarnings = 0 - maxErrors = 0 - }''') - - testProject = testProject.withToolsConfig(detektConfiguration(testProject, Fixtures.Detekt.SOURCES_WITH_ERRORS)) - - TestProject.Result result = testProject + def result = createProjectWithZeroThreshold(Fixtures.Detekt.SOURCES_WITH_ERRORS) .buildAndFail('check') assertThat(result.logs).containsLimitExceeded(1, 0) @@ -74,33 +54,15 @@ class DetektIntegrationTest { @Test void shouldNotFailWhenDetektIsNotConfigured() throws Exception { - def testProject = projectRule.newProject() - .withPlugin("io.gitlab.arturbosch.detekt", "1.0.0.RC6-2") - .withSourceSet('main', Fixtures.Detekt.SOURCES_WITH_WARNINGS) - .withPenalty('''{ - maxWarnings = 0 - maxErrors = 0 - }''') + def result = createProjectWithoutDetekt() .build('check') - TestProject.Result result = testProject - assertThat(result.logs).doesNotContainDetektViolations() } @Test void shouldNotFailWhenWarningsAreWithinThreshold() throws Exception { - def testProject = projectRule.newProject() - .withPlugin("io.gitlab.arturbosch.detekt", "1.0.0.RC6-2") - .withSourceSet('main', Fixtures.Detekt.SOURCES_WITH_WARNINGS) - .withPenalty('''{ - maxWarnings = 1 - maxErrors = 0 - }''') - - testProject = testProject.withToolsConfig(detektConfiguration(testProject, Fixtures.Detekt.SOURCES_WITH_WARNINGS)) - - TestProject.Result result = testProject + def result = createProjectWith(Fixtures.Detekt.SOURCES_WITH_WARNINGS, 1, 0) .build('check') assertThat(result.logs).containsDetektViolations(0, 1, @@ -109,17 +71,7 @@ class DetektIntegrationTest { @Test void shouldNotFailWhenErrorsAreWithinThreshold() throws Exception { - def testProject = projectRule.newProject() - .withPlugin("io.gitlab.arturbosch.detekt", "1.0.0.RC6-2") - .withSourceSet('main', Fixtures.Detekt.SOURCES_WITH_WARNINGS) - .withPenalty('''{ - maxWarnings = 0 - maxErrors = 1 - }''') - - testProject = testProject.withToolsConfig(detektConfiguration(testProject, Fixtures.Detekt.SOURCES_WITH_ERRORS)) - - TestProject.Result result = testProject + def result = createProjectWith(Fixtures.Detekt.SOURCES_WITH_ERRORS, 0, 1) .build('check') assertThat(result.logs).containsDetektViolations(1, 0, @@ -159,6 +111,32 @@ class DetektIntegrationTest { assertThat(result.logs).containsDetektNotApplied() } + private TestProject createProjectWithZeroThreshold(File sources) { + createProjectWith(sources) + } + + private TestProject createProjectWith(File sources, int maxWarnings = 0, int maxErrors = 0) { + def testProject = projectRule.newProject() + .withPlugin("io.gitlab.arturbosch.detekt", "1.0.0.RC6-2") + .withSourceSet('main', sources) + .withPenalty("""{ + maxWarnings = ${maxWarnings} + maxErrors = ${maxErrors} + }""") + + testProject.withToolsConfig(detektConfiguration(testProject, sources)) + } + + private TestProject createProjectWithoutDetekt() { + projectRule.newProject() + .withPlugin("io.gitlab.arturbosch.detekt", "1.0.0.RC6-2") + .withSourceSet('main', Fixtures.Detekt.SOURCES_WITH_WARNINGS) + .withPenalty('''{ + maxWarnings = 0 + maxErrors = 0 + }''') + } + private static GString detektConfiguration(TestProject testProject, File input) { """ detekt { From 71b32c5976963889183bb9a3ccddeb8a73d27789 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Thu, 25 Jan 2018 16:31:20 +0100 Subject: [PATCH 46/81] Refer to detekt dokumentation on how to apply the plugin --- .../staticanalysis/internal/detekt/DetektConfigurator.groovy | 2 +- plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index 66593b5..bbf8365 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -11,7 +11,7 @@ import org.gradle.api.Task class DetektConfigurator implements Configurator { private static final String DETEKT_PLUGIN = 'io.gitlab.arturbosch.detekt' - private static final String DETEKT_NOT_APPLIED = 'The detekt plugin is configured but not applied. Please apply the plugin in your build script.' + private static final String DETEKT_NOT_APPLIED = 'The detekt plugin is configured but not applied. Please apply the plugin in your build script For more information see https://github.com/arturbosch/detekt.' private final Project project private final Violations violations diff --git a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy index cdeb87a..a5a4cdd 100644 --- a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy @@ -13,7 +13,7 @@ import static com.novoda.test.TestProject.Result.Logs; class LogsSubject extends Subject { private static final String VIOLATIONS_LIMIT_EXCEEDED = "Violations limit exceeded" - private static final String DETEKT_NOT_APPLIED = "The detekt plugin is configured but not applied. Please apply the plugin in your build script." + private static final String DETEKT_NOT_APPLIED = "The detekt plugin is configured but not applied. Please apply the plugin in your build script For more information see https://github.com/arturbosch/detekt." private static final String CHECKSTYLE_VIOLATIONS_FOUND = "Checkstyle violations found" private static final String PMD_VIOLATIONS_FOUND = "PMD violations found" private static final String FINDBUGS_VIOLATIONS_FOUND = "Findbugs violations found" From 4292d6b091dccabc237199056f4e2e9101aecb34 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Thu, 25 Jan 2018 18:10:22 +0100 Subject: [PATCH 47/81] Create TestAndroidKotlin project which can be used to verify the detekt integration works as well with android --- .../test/TestAndroidKotlinProject.groovy | 78 +++++++++++++++++++ .../com/novoda/test/TestProjectRule.groovy | 4 + 2 files changed, 82 insertions(+) create mode 100644 plugin/src/test/groovy/com/novoda/test/TestAndroidKotlinProject.groovy diff --git a/plugin/src/test/groovy/com/novoda/test/TestAndroidKotlinProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestAndroidKotlinProject.groovy new file mode 100644 index 0000000..9dcdf96 --- /dev/null +++ b/plugin/src/test/groovy/com/novoda/test/TestAndroidKotlinProject.groovy @@ -0,0 +1,78 @@ +package com.novoda.test + +class TestAndroidKotlinProject extends TestProject { + private static final Closure TEMPLATE = { TestAndroidKotlinProject project -> + """ +buildscript { + repositories { + google() + jcenter() + } + dependencies { + classpath 'com.android.tools.build:gradle:3.0.1' + classpath 'org.jetbrains.kotlin:kotlin-gradle-plugin:1.2.20' + } +} +plugins { + ${formatPlugins(project)} +} +repositories { + google() + jcenter() +} +apply plugin: 'com.android.library' +apply plugin: 'kotlin-android' + +android { + compileSdkVersion 27 + buildToolsVersion '27.0.0' + + defaultConfig { + minSdkVersion 16 + targetSdkVersion 27 + versionCode 1 + versionName '1.0' + } + sourceSets { + ${formatSourceSets(project)} + } + ${project.additionalAndroidConfig} +} +${formatExtension(project)} +""" + } + + private String additionalAndroidConfig = '' + + TestAndroidKotlinProject() { + super(TEMPLATE) + File localProperties = Fixtures.LOCAL_PROPERTIES + if (localProperties.exists()) { + withFile(localProperties, 'local.properties') + } + } + + private static String formatSourceSets(TestProject project) { + project.sourceSets + .entrySet() + .collect { Map.Entry> entry -> + """$entry.key { + manifest.srcFile '${Fixtures.ANDROID_MANIFEST}' + java { + ${entry.value.collect { "srcDir '$it'" }.join('\n\t\t\t\t')} + } + }""" + } + .join('\n\t\t') + } + + @Override + List defaultArguments() { + ['-x', 'lint'] + super.defaultArguments() + } + + TestAndroidKotlinProject withAdditionalAndroidConfig(String additionalAndroidConfig) { + this.additionalAndroidConfig = additionalAndroidConfig + return this + } +} diff --git a/plugin/src/test/groovy/com/novoda/test/TestProjectRule.groovy b/plugin/src/test/groovy/com/novoda/test/TestProjectRule.groovy index ec32aeb..ede1f69 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestProjectRule.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestProjectRule.groovy @@ -19,6 +19,10 @@ final class TestProjectRule implements TestRule { new TestProjectRule({ new TestAndroidProject() }, { String name -> "project.android.sourceSets.$name" }, 'Android project') } + static TestProjectRule forAndroidKotlinProject() { + new TestProjectRule({ new TestAndroidKotlinProject() }, { String name -> "project.android.sourceSets.$name" }, 'Android project') + } + static TestProjectRule forKotlinProject() { new TestProjectRule({ new TestKotlinProject() }, { String name -> "project.sourceSets.$name" }, 'Kotlin project') } From 0f36aa984591b494342f7f5ba339b3c1fe9be083 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Thu, 25 Jan 2018 18:10:47 +0100 Subject: [PATCH 48/81] Verify all tests are working as well for a android project using kotlin --- .../staticanalysis/internal/detekt/DetektIntegrationTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy index a497dec..f663a3e 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy @@ -22,7 +22,7 @@ class DetektIntegrationTest { @Parameterized.Parameters(name = "{0}") static Iterable rules() { - return [TestProjectRule.forKotlinProject()] + return [TestProjectRule.forKotlinProject(), TestProjectRule.forAndroidKotlinProject()] } @Rule From a8a3f5234dda81aa61a8b0028005bf0b0d4c32b8 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Thu, 25 Jan 2018 18:16:32 +0100 Subject: [PATCH 49/81] Name the source set folder to kotklin since it's a kotlin project --- .../test/groovy/com/novoda/test/TestAndroidKotlinProject.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/test/groovy/com/novoda/test/TestAndroidKotlinProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestAndroidKotlinProject.groovy index 9dcdf96..1e1b48b 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestAndroidKotlinProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestAndroidKotlinProject.groovy @@ -58,7 +58,7 @@ ${formatExtension(project)} .collect { Map.Entry> entry -> """$entry.key { manifest.srcFile '${Fixtures.ANDROID_MANIFEST}' - java { + kotlin { ${entry.value.collect { "srcDir '$it'" }.join('\n\t\t\t\t')} } }""" From f717db06d9947587a88b5b0c3116031f2045b74c Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Fri, 26 Jan 2018 10:00:23 +0100 Subject: [PATCH 50/81] Change label for android kotlin test project --- plugin/src/test/groovy/com/novoda/test/TestProjectRule.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/test/groovy/com/novoda/test/TestProjectRule.groovy b/plugin/src/test/groovy/com/novoda/test/TestProjectRule.groovy index ede1f69..f25fdf1 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestProjectRule.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestProjectRule.groovy @@ -20,7 +20,7 @@ final class TestProjectRule implements TestRule { } static TestProjectRule forAndroidKotlinProject() { - new TestProjectRule({ new TestAndroidKotlinProject() }, { String name -> "project.android.sourceSets.$name" }, 'Android project') + new TestProjectRule({ new TestAndroidKotlinProject() }, { String name -> "project.android.sourceSets.$name" }, 'Android kotlin project') } static TestProjectRule forKotlinProject() { From 72bd9fac0ab970d756de09f973f8f4a153a87f7d Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Fri, 26 Jan 2018 11:21:15 +0100 Subject: [PATCH 51/81] Remove nasty whitespaces --- .../com/novoda/test/TestAndroidKotlinProject.groovy | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugin/src/test/groovy/com/novoda/test/TestAndroidKotlinProject.groovy b/plugin/src/test/groovy/com/novoda/test/TestAndroidKotlinProject.groovy index 1e1b48b..eb9ffaf 100644 --- a/plugin/src/test/groovy/com/novoda/test/TestAndroidKotlinProject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/TestAndroidKotlinProject.groovy @@ -4,23 +4,23 @@ class TestAndroidKotlinProject extends TestProject { private static final Closure TEMPLATE = { TestAndroidKotlinProject project -> """ buildscript { - repositories { + repositories { google() jcenter() } dependencies { - classpath 'com.android.tools.build:gradle:3.0.1' + classpath 'com.android.tools.build:gradle:3.0.1' classpath 'org.jetbrains.kotlin:kotlin-gradle-plugin:1.2.20' } } plugins { ${formatPlugins(project)} } -repositories { +repositories { google() jcenter() } -apply plugin: 'com.android.library' +apply plugin: 'com.android.library' apply plugin: 'kotlin-android' android { From 0d734d0583e3d6d1274d93ac05dd672ff4df21c8 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Fri, 26 Jan 2018 14:03:21 +0100 Subject: [PATCH 52/81] Use relative path with project dir to link detekt reports --- .../staticanalysis/internal/detekt/DetektConfigurator.groovy | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index bbf8365..e7f46c7 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -66,9 +66,8 @@ class DetektConfigurator implements Configurator { private CollectDetektViolationsTask createCollectViolationsTask(Violations violations) { def reportFilePath = "${project.extensions.findByName('detekt').systemOrDefaultProfile().output}/detekt-checkstyle.xml" - project.tasks.create("collectDetektViolations", CollectDetektViolationsTask) { collectViolations -> - collectViolations.xmlReportFile = new File(reportFilePath) + collectViolations.xmlReportFile = project.file(reportFilePath) collectViolations.violations = violations } } From 4eb544e6558a454219a347e672ea76ad1cf3f084 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Fri, 26 Jan 2018 14:14:57 +0100 Subject: [PATCH 53/81] Make htmlReportFile configurable since detekt names it different than the xml output --- .../staticanalysis/internal/CollectViolationsTask.groovy | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/CollectViolationsTask.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/CollectViolationsTask.groovy index 56f1cb2..4e427a6 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/CollectViolationsTask.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/CollectViolationsTask.groovy @@ -6,6 +6,7 @@ import org.gradle.api.tasks.TaskAction abstract class CollectViolationsTask extends DefaultTask { private File xmlReportFile + private File htmlReportFile private Violations violations CollectViolationsTask() { @@ -16,6 +17,10 @@ abstract class CollectViolationsTask extends DefaultTask { this.xmlReportFile = xmlReportFile } + void setHtmlReportFile(File htmlReportFile) { + this.htmlReportFile = htmlReportFile + } + void setViolations(Violations violations) { this.violations = violations } @@ -30,7 +35,7 @@ abstract class CollectViolationsTask extends DefaultTask { } File getHtmlReportFile() { - new File(xmlReportFile.absolutePath - '.xml' + '.html') + htmlReportFile ?: new File(xmlReportFile.absolutePath - '.xml' + '.html') } abstract void collectViolations(File xmlReportFile, File htmlReportFile, Violations violations) From 9e7061a323a0979ce87395c27322f05f48d16510 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Fri, 26 Jan 2018 14:15:28 +0100 Subject: [PATCH 54/81] Link to correct detekt html report --- .../internal/detekt/DetektConfigurator.groovy | 5 +++-- .../internal/detekt/DetektIntegrationTest.groovy | 8 ++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index e7f46c7..30f6fd1 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -65,9 +65,10 @@ class DetektConfigurator implements Configurator { } private CollectDetektViolationsTask createCollectViolationsTask(Violations violations) { - def reportFilePath = "${project.extensions.findByName('detekt').systemOrDefaultProfile().output}/detekt-checkstyle.xml" + def outputFolder = project.file(project.extensions.findByName('detekt').systemOrDefaultProfile().output) project.tasks.create("collectDetektViolations", CollectDetektViolationsTask) { collectViolations -> - collectViolations.xmlReportFile = project.file(reportFilePath) + collectViolations.xmlReportFile = new File(outputFolder, 'detekt-checkstyle.xml') + collectViolations.htmlReportFile = new File(outputFolder, 'detekt-report.html') collectViolations.violations = violations } } diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy index f663a3e..c96a595 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy @@ -39,7 +39,7 @@ class DetektIntegrationTest { assertThat(result.logs).containsLimitExceeded(0, 1) assertThat(result.logs).containsDetektViolations(0, 1, - result.buildFileUrl('reports/detekt-checkstyle.html')) + result.buildFileUrl('reports/detekt-report.html')) } @Test @@ -49,7 +49,7 @@ class DetektIntegrationTest { assertThat(result.logs).containsLimitExceeded(1, 0) assertThat(result.logs).containsDetektViolations(1, 0, - result.buildFileUrl('reports/detekt-checkstyle.html')) + result.buildFileUrl('reports/detekt-report.html')) } @Test @@ -66,7 +66,7 @@ class DetektIntegrationTest { .build('check') assertThat(result.logs).containsDetektViolations(0, 1, - result.buildFileUrl('reports/detekt-checkstyle.html')) + result.buildFileUrl('reports/detekt-report.html')) } @Test @@ -75,7 +75,7 @@ class DetektIntegrationTest { .build('check') assertThat(result.logs).containsDetektViolations(1, 0, - result.buildFileUrl('reports/detekt-checkstyle.html')) + result.buildFileUrl('reports/detekt-report.html')) } @Test From 318fb7954e8f3aa72b60b74dbbcfa263c139daa0 Mon Sep 17 00:00:00 2001 From: Tobias Heine Date: Fri, 26 Jan 2018 14:38:27 +0100 Subject: [PATCH 55/81] Use getter since we now have a field for the htmlReportFile --- .../novoda/staticanalysis/internal/CollectViolationsTask.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/CollectViolationsTask.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/CollectViolationsTask.groovy index 4e427a6..987fded 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/CollectViolationsTask.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/CollectViolationsTask.groovy @@ -27,7 +27,7 @@ abstract class CollectViolationsTask extends DefaultTask { @TaskAction final void run() { - collectViolations(xmlReportFile, htmlReportFile, violations) + collectViolations(getXmlReportFile(), getHtmlReportFile(), violations) } File getXmlReportFile() { From 3e91310fa7a0bab060f9f9219d27223909207b7a Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Mon, 29 Jan 2018 17:10:24 +0000 Subject: [PATCH 56/81] Improve readme file --- README.md | 114 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 67 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index 8fc4878..9eec7bb 100644 --- a/README.md +++ b/README.md @@ -1,23 +1,40 @@ -# gradle-static-analysis-plugin -[![](https://ci.novoda.com/buildStatus/icon?job=gradle-static-analysis-plugin)](https://ci.novoda.com/job/gradle-static-analysis-plugin/lastSuccessfulBuild/console) [![](https://img.shields.io/badge/License-Apache%202.0-lightgrey.svg)](LICENSE.txt) [![Bintray](https://api.bintray.com/packages/novoda/maven/gradle-static-analysis-plugin/images/download.svg) ](https://bintray.com/novoda/maven/gradle-static-analysis-plugin/_latestVersion) +# Gradle static analysis plugin +[![](https://ci.novoda.com/buildStatus/icon?job=gradle-static-analysis-plugin)](https://ci.novoda.com/job/gradle-static-analysis-plugin/lastSuccessfulBuild) [![](https://img.shields.io/badge/License-Apache%202.0-lightgrey.svg)](LICENSE.txt) [![Bintray](https://api.bintray.com/packages/novoda/maven/gradle-static-analysis-plugin/images/download.svg) ](https://bintray.com/novoda/maven/gradle-static-analysis-plugin/_latestVersion) -A Gradle plugin to easily apply the same setup of static analysis tools across different Android or Java projects.
+A Gradle plugin to easily apply the same setup of static analysis tools across different Android or Java projects. ## Description - -Gradle supports many popular static analysis (Checkstyle, PMD, FindBugs, etc) via a set of built-in -plugins. Using these plugins in an Android module will require an additional setup to compensate for the differences between -the model adopted by the Android plugin compared to the the Java one.
+Gradle supports many popular static analysis (Checkstyle, PMD, FindBugs, etc) via a set of built-in plugins. +Using these plugins in an Android module will require an additional setup to compensate for the differences between +the model adopted by the Android plugin compared to the the Java one. The `gradle-static-analysis-plugin` aims to provide: -- flexible, configurable penalty strategy for builds, -- easy, Android-friendly integration for all static analysis, -- convenient way of sharing same setup across different projects, -- healthy, versionable and configurable defaults. +- flexible, configurable penalty strategy for builds +- easy, Android-friendly integration for all static analysis +- convenient way of sharing same setup across different projects +- healthy, versionable and configurable defaults + +### Suported static analysis tools + +Tool | Java | Android (Java) | Kotlin and
Android (Kotlin) +---- | -------- | -------- | ----- +[`Checkstyle`](https://checkstyle.sourceforge.net) | :white_check_mark: | :white_check_mark: | ✖️ +[`PMD`](https://pmd.github.io) | :white_check_mark: | :white_check_mark: | ✖️ +[`FindBugs`](http://findbugs.sourceforge.net/) | :white_check_mark: | :white_check_mark: | ✖️ +[`Detekt`](https://github.com/arturbosch/detekt) | ✖️ | ✖️ | :white_check_mark: | ✖️ +[`KtLint`\*](https://github.com/shyiko/ktlint) | ✖️ | ✖️ | ✖️ +[`Android Lint`\*](https://developer.android.com/studio/write/lint.html) | ✖️ | ✖️ | ✖️ + +_\* Not supported [yet](https://github.com/novoda/gradle-static-analysis-plugin/issues?q=is%3Aopen+is%3Aissue+label%3A%22new+tool%22)_ + +### Out-of-the-box support for Android and Kotlin projects +Android projects use a Gradle model that is not compatible with the Java one, supported by the built-in static analysis tools plugins. +Applying `gradle-static-analysis-plugin` to your Android project will make sure all the necessary tasks are created and correctly configured +without any additional hassle. -## Adding to your project +## Add the plugin to your project +The plugin is available on jCenter and can be included as a classpath dependency: -The plugin is released in jcenter and can be included as a classpath dependency: ```gradle buildscript { repositories { @@ -28,14 +45,16 @@ buildscript { } } ``` -and then apply the plugin via: + +You can then apply the plugin via: + ```gradle apply plugin: 'com.novoda.static-analysis' ``` ## Simple usage - A typical configuration for the plugin will look like: + ```gradle staticAnalysis { penalty { @@ -48,12 +67,15 @@ staticAnalysis { pmd { ruleSetFiles = project.files('path/to/rules.xml') } - findbugs {} + findbugs { + // ... + } } ``` #### Configurable thresholds -Users can define maximum amount of warnings and errors tolerated in a build via the gradle configuration: +Users can define maximum amount of warnings and errors tolerated in a build via the Gradle configuration: + ```gradle staticAnalysis { penalty { @@ -62,12 +84,17 @@ staticAnalysis { } } ``` + Violations are then collected while running all the static analysis tools enabled in the project and split between errors and warnings. -Only in the end they are cumulatively evaluated against the thresholds provided in the configuration to decide whether the build should fail or not. +Only in the end they are cumulatively evaluated against the thresholds provided in the configuration to decide whether the build should +fail or not. + +## Advanced configuration -#### Better output +### Improve the report with a base URL Build logs will show an overall report of how many violations have been found during the analysis and the links to -the relevant html reports, for instance: +the relevant HTML reports, for instance: + ``` > PMD rule violations were found (2 errors, 2 warnings). See the reports at: - file:///foo/project/build/reports/pmd/main.html @@ -77,8 +104,9 @@ the relevant html reports, for instance: ``` It's possible to specify a custom renderer for the report urls in the logs via the `logs` extension. This can be useful in CI - environments, where the local paths are not reachable directly. For instance the snippet below will replace the base url with - one of your choice: +environments, where the local paths are not reachable directly. For instance the snippet below will replace the base URL with +one mapping to an hypothetical Jenkins workspace: + ```gradle staticAnalysis { ... @@ -87,22 +115,22 @@ staticAnalysis { } } ``` -so that in the logs you will see the report urls printed as + +This way, in the CI logs you will see the report URLs printed as: + ``` > Checkstyle rule violations were found (0 errors, 1 warnings). See the reports at: - http://ci.mycompany.com/job/myproject/ws/app/build/reports/checkstyle/main.html ``` -More info on the topic can be found in the [`LogsExtension`](https://github.com/novoda/gradle-static-analysis-plugin/blob/master/plugin/src/main/groovy/com/novoda/staticanalysis/LogsExtension.groovy) -groovydoc. -#### Out-of-the-box support for Android projects -Android projects use a gradle model that is not compatible with the Java one, supported by the built-in static analysis tools plugins. -Applying `gradle-static-analysis-plugin` to your Android project will make sure all the necessary tasks are created and correctly configured -without any additional hassle. +And that will make them easier to follow them to the respective reports. More info on the topic can be found in the +[`LogsExtension`](blob/master/plugin/src/main/groovy/com/novoda/staticanalysis/LogsExtension.groovy) +Groovydocs. #### Support for `exclude` filters You can specify custom patterns to exclude specific files from the static analysis. All you have to do is to specify `exclude` in the configuration of your tool of choice: + ```gradle staticAnalysis { findbugs { @@ -116,9 +144,10 @@ staticAnalysis { #### Support for Android variants Sometimes using `exclude` filters could be not enough. When using the plugin in an Android project you may want to consider -only one specific variant as part of the analysis. The plugin provides a way of defining which Android variant should be included -via the `includeVariants` method added to each tool extension. Eg: -``` +only one specific variant as part of the analysis. The plugin provides a way of defining which Android variants should be included +via the `includeVariants` method added to each tool extension. E.g., + +```gradle staticAnalysis { findbugs { includeVariants { variant -> @@ -128,19 +157,10 @@ staticAnalysis { } ``` -### Current status / Roadmap - -The plugin is **under early development** and to be considered in pre-alpha stage. - -#### Static analysis tools supported - -Tool | Android | Java | Documentation | -:----:|:--------:|:--------:|:----:| -`Checkstyle` | :white_check_mark: | :white_check_mark: | _Coming Soon_ | -`PMD` | :white_check_mark: | :white_check_mark: | _Coming Soon_ | -`FindBugs` | :white_check_mark: | :white_check_mark: | _Coming Soon_ | - -#### Support for sharable configurations +## Roadmap +The plugin is under active development and to be considered in **beta stage**. It is routinely used by many Novoda projects and +by other external projects with no known critical issues. The API is supposed to be relatively stable, but there still may be +breaking changes as we move towards version 1.0. -The plugin can consume rules (eg: configuration files for Checkstyle or PMD, default exclude filters, etc) via a -separate artifact you can share across projects. +Future improvements can be found on the repository's +[issue tracker](https://github.com/novoda/gradle-static-analysis-plugin/issues?q=is%3Aopen+is%3Aissue+label%3Aenhancement). From d453d9cf7ea29bcb9a55d91e69908b218faa762c Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Tue, 30 Jan 2018 14:59:55 +0000 Subject: [PATCH 57/81] Fix Apache 2.0 licence --- LICENSE | 177 ++++++++++++++++++++++++++++++++++++++++++++++++ LICENSE.txt | 191 ---------------------------------------------------- NOTICE | 13 ++++ 3 files changed, 190 insertions(+), 191 deletions(-) create mode 100644 LICENSE delete mode 100644 LICENSE.txt create mode 100644 NOTICE diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..0561f16 --- /dev/null +++ b/LICENSE @@ -0,0 +1,177 @@ + + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + +TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + +1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + +2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + +3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + +4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + +5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + +6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + +7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + +8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + +9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + +END OF TERMS AND CONDITIONS \ No newline at end of file diff --git a/LICENSE.txt b/LICENSE.txt deleted file mode 100644 index 58c3005..0000000 --- a/LICENSE.txt +++ /dev/null @@ -1,191 +0,0 @@ - Copyright 2014 Novoda Ltd - - 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. - - - Apache License - Version 2.0, January 2004 - http://www.apache.org/licenses/ - - TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION - - 1. Definitions. - - "License" shall mean the terms and conditions for use, reproduction, - and distribution as defined by Sections 1 through 9 of this document. - - "Licensor" shall mean the copyright owner or entity authorized by - the copyright owner that is granting the License. - - "Legal Entity" shall mean the union of the acting entity and all - other entities that control, are controlled by, or are under common - control with that entity. For the purposes of this definition, - "control" means (i) the power, direct or indirect, to cause the - direction or management of such entity, whether by contract or - otherwise, or (ii) ownership of fifty percent (50%) or more of the - outstanding shares, or (iii) beneficial ownership of such entity. - - "You" (or "Your") shall mean an individual or Legal Entity - exercising permissions granted by this License. - - "Source" form shall mean the preferred form for making modifications, - including but not limited to software source code, documentation - source, and configuration files. - - "Object" form shall mean any form resulting from mechanical - transformation or translation of a Source form, including but - not limited to compiled object code, generated documentation, - and conversions to other media types. - - "Work" shall mean the work of authorship, whether in Source or - Object form, made available under the License, as indicated by a - copyright notice that is included in or attached to the work - (an example is provided in the Appendix below). - - "Derivative Works" shall mean any work, whether in Source or Object - form, that is based on (or derived from) the Work and for which the - editorial revisions, annotations, elaborations, or other modifications - represent, as a whole, an original work of authorship. For the purposes - of this License, Derivative Works shall not include works that remain - separable from, or merely link (or bind by name) to the interfaces of, - the Work and Derivative Works thereof. - - "Contribution" shall mean any work of authorship, including - the original version of the Work and any modifications or additions - to that Work or Derivative Works thereof, that is intentionally - submitted to Licensor for inclusion in the Work by the copyright owner - or by an individual or Legal Entity authorized to submit on behalf of - the copyright owner. For the purposes of this definition, "submitted" - means any form of electronic, verbal, or written communication sent - to the Licensor or its representatives, including but not limited to - communication on electronic mailing lists, source code control systems, - and issue tracking systems that are managed by, or on behalf of, the - Licensor for the purpose of discussing and improving the Work, but - excluding communication that is conspicuously marked or otherwise - designated in writing by the copyright owner as "Not a Contribution." - - "Contributor" shall mean Licensor and any individual or Legal Entity - on behalf of whom a Contribution has been received by Licensor and - subsequently incorporated within the Work. - - 2. Grant of Copyright License. Subject to the terms and conditions of - this License, each Contributor hereby grants to You a perpetual, - worldwide, non-exclusive, no-charge, royalty-free, irrevocable - copyright license to reproduce, prepare Derivative Works of, - publicly display, publicly perform, sublicense, and distribute the - Work and such Derivative Works in Source or Object form. - - 3. Grant of Patent License. Subject to the terms and conditions of - this License, each Contributor hereby grants to You a perpetual, - worldwide, non-exclusive, no-charge, royalty-free, irrevocable - (except as stated in this section) patent license to make, have made, - use, offer to sell, sell, import, and otherwise transfer the Work, - where such license applies only to those patent claims licensable - by such Contributor that are necessarily infringed by their - Contribution(s) alone or by combination of their Contribution(s) - with the Work to which such Contribution(s) was submitted. If You - institute patent litigation against any entity (including a - cross-claim or counterclaim in a lawsuit) alleging that the Work - or a Contribution incorporated within the Work constitutes direct - or contributory patent infringement, then any patent licenses - granted to You under this License for that Work shall terminate - as of the date such litigation is filed. - - 4. Redistribution. You may reproduce and distribute copies of the - Work or Derivative Works thereof in any medium, with or without - modifications, and in Source or Object form, provided that You - meet the following conditions: - - (a) You must give any other recipients of the Work or - Derivative Works a copy of this License; and - - (b) You must cause any modified files to carry prominent notices - stating that You changed the files; and - - (c) You must retain, in the Source form of any Derivative Works - that You distribute, all copyright, patent, trademark, and - attribution notices from the Source form of the Work, - excluding those notices that do not pertain to any part of - the Derivative Works; and - - (d) If the Work includes a "NOTICE" text file as part of its - distribution, then any Derivative Works that You distribute must - include a readable copy of the attribution notices contained - within such NOTICE file, excluding those notices that do not - pertain to any part of the Derivative Works, in at least one - of the following places: within a NOTICE text file distributed - as part of the Derivative Works; within the Source form or - documentation, if provided along with the Derivative Works; or, - within a display generated by the Derivative Works, if and - wherever such third-party notices normally appear. The contents - of the NOTICE file are for informational purposes only and - do not modify the License. You may add Your own attribution - notices within Derivative Works that You distribute, alongside - or as an addendum to the NOTICE text from the Work, provided - that such additional attribution notices cannot be construed - as modifying the License. - - You may add Your own copyright statement to Your modifications and - may provide additional or different license terms and conditions - for use, reproduction, or distribution of Your modifications, or - for any such Derivative Works as a whole, provided Your use, - reproduction, and distribution of the Work otherwise complies with - the conditions stated in this License. - - 5. Submission of Contributions. Unless You explicitly state otherwise, - any Contribution intentionally submitted for inclusion in the Work - by You to the Licensor shall be under the terms and conditions of - this License, without any additional terms or conditions. - Notwithstanding the above, nothing herein shall supersede or modify - the terms of any separate license agreement you may have executed - with Licensor regarding such Contributions. - - 6. Trademarks. This License does not grant permission to use the trade - names, trademarks, service marks, or product names of the Licensor, - except as required for reasonable and customary use in describing the - origin of the Work and reproducing the content of the NOTICE file. - - 7. Disclaimer of Warranty. Unless required by applicable law or - agreed to in writing, Licensor provides the Work (and each - Contributor provides its Contributions) on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or - implied, including, without limitation, any warranties or conditions - of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A - PARTICULAR PURPOSE. You are solely responsible for determining the - appropriateness of using or redistributing the Work and assume any - risks associated with Your exercise of permissions under this License. - - 8. Limitation of Liability. In no event and under no legal theory, - whether in tort (including negligence), contract, or otherwise, - unless required by applicable law (such as deliberate and grossly - negligent acts) or agreed to in writing, shall any Contributor be - liable to You for damages, including any direct, indirect, special, - incidental, or consequential damages of any character arising as a - result of this License or out of the use or inability to use the - Work (including but not limited to damages for loss of goodwill, - work stoppage, computer failure or malfunction, or any and all - other commercial damages or losses), even if such Contributor - has been advised of the possibility of such damages. - - 9. Accepting Warranty or Additional Liability. While redistributing - the Work or Derivative Works thereof, You may choose to offer, - and charge a fee for, acceptance of support, warranty, indemnity, - or other liability obligations and/or rights consistent with this - License. However, in accepting such obligations, You may act only - on Your own behalf and on Your sole responsibility, not on behalf - of any other Contributor, and only if You agree to indemnify, - defend, and hold each Contributor harmless for any liability - incurred by, or claims asserted against, such Contributor by reason - of your accepting any such warranty or additional liability. - - END OF TERMS AND CONDITIONS diff --git a/NOTICE b/NOTICE new file mode 100644 index 0000000..cca3f3f --- /dev/null +++ b/NOTICE @@ -0,0 +1,13 @@ +Copyright 2018 Novoda Ltd. + +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. \ No newline at end of file From 21d35336e9fd31113b37073db6ec2fa087862d6d Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Tue, 30 Jan 2018 15:02:13 +0000 Subject: [PATCH 58/81] Move non-essential content out of the readme --- README.md | 84 ------------------------------------------ docs/advanced-usage.md | 0 2 files changed, 84 deletions(-) create mode 100644 docs/advanced-usage.md diff --git a/README.md b/README.md index 9eec7bb..6dc6b90 100644 --- a/README.md +++ b/README.md @@ -73,90 +73,6 @@ staticAnalysis { } ``` -#### Configurable thresholds -Users can define maximum amount of warnings and errors tolerated in a build via the Gradle configuration: - -```gradle -staticAnalysis { - penalty { - maxErrors = 10 - maxWarnings = 10 - } -} -``` - -Violations are then collected while running all the static analysis tools enabled in the project and split between errors and warnings. -Only in the end they are cumulatively evaluated against the thresholds provided in the configuration to decide whether the build should -fail or not. - -## Advanced configuration - -### Improve the report with a base URL -Build logs will show an overall report of how many violations have been found during the analysis and the links to -the relevant HTML reports, for instance: - -``` - > PMD rule violations were found (2 errors, 2 warnings). See the reports at: - - file:///foo/project/build/reports/pmd/main.html - - file:///foo/project/build/reports/pmd/main2.html - - file:///foo/project/build/reports/pmd/main3.html - - file:///foo/project/build/reports/pmd/main4.html -``` - -It's possible to specify a custom renderer for the report urls in the logs via the `logs` extension. This can be useful in CI -environments, where the local paths are not reachable directly. For instance the snippet below will replace the base URL with -one mapping to an hypothetical Jenkins workspace: - -```gradle -staticAnalysis { - ... - logs { - reportBaseUrl "http://ci.mycompany.com/job/myproject/ws/app/build/reports" - } -} -``` - -This way, in the CI logs you will see the report URLs printed as: - -``` -> Checkstyle rule violations were found (0 errors, 1 warnings). See the reports at: -- http://ci.mycompany.com/job/myproject/ws/app/build/reports/checkstyle/main.html -``` - -And that will make them easier to follow them to the respective reports. More info on the topic can be found in the -[`LogsExtension`](blob/master/plugin/src/main/groovy/com/novoda/staticanalysis/LogsExtension.groovy) -Groovydocs. - -#### Support for `exclude` filters -You can specify custom patterns to exclude specific files from the static analysis. All you have to do is to specify `exclude` -in the configuration of your tool of choice: - -```gradle -staticAnalysis { - findbugs { - exclude '**/*Test.java' // file pattern - exclude project.fileTree('src/test/java') // entire folder - exclude project.file('src/main/java/foo/bar/Constants.java') // specific file - exclude project.sourceSets.main.java.srcDirs // entire source set - } -} -``` - -#### Support for Android variants -Sometimes using `exclude` filters could be not enough. When using the plugin in an Android project you may want to consider -only one specific variant as part of the analysis. The plugin provides a way of defining which Android variants should be included -via the `includeVariants` method added to each tool extension. E.g., - -```gradle -staticAnalysis { - findbugs { - includeVariants { variant -> - variant.name.equals('debug') // only the debug variant - } - } -} -``` - ## Roadmap The plugin is under active development and to be considered in **beta stage**. It is routinely used by many Novoda projects and by other external projects with no known critical issues. The API is supposed to be relatively stable, but there still may be diff --git a/docs/advanced-usage.md b/docs/advanced-usage.md new file mode 100644 index 0000000..e69de29 From 8cf0a9e3bbade126dba0c4bab882d7f774381389 Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Tue, 30 Jan 2018 15:02:27 +0000 Subject: [PATCH 59/81] Clarify supported tools table in README --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 6dc6b90..70e2ac6 100644 --- a/README.md +++ b/README.md @@ -18,11 +18,11 @@ The `gradle-static-analysis-plugin` aims to provide: Tool | Java | Android (Java) | Kotlin and
Android (Kotlin) ---- | -------- | -------- | ----- -[`Checkstyle`](https://checkstyle.sourceforge.net) | :white_check_mark: | :white_check_mark: | ✖️ -[`PMD`](https://pmd.github.io) | :white_check_mark: | :white_check_mark: | ✖️ -[`FindBugs`](http://findbugs.sourceforge.net/) | :white_check_mark: | :white_check_mark: | ✖️ -[`Detekt`](https://github.com/arturbosch/detekt) | ✖️ | ✖️ | :white_check_mark: | ✖️ -[`KtLint`\*](https://github.com/shyiko/ktlint) | ✖️ | ✖️ | ✖️ +[`Checkstyle`](https://checkstyle.sourceforge.net) | :white_check_mark: | :white_check_mark: | — +[`PMD`](https://pmd.github.io) | :white_check_mark: | :white_check_mark: | — +[`FindBugs`](http://findbugs.sourceforge.net/) | :white_check_mark: | :white_check_mark: | — +[`Detekt`](https://github.com/arturbosch/detekt) | — | — | :white_check_mark: +[`KtLint`\*](https://github.com/shyiko/ktlint) | — | — | ✖️ [`Android Lint`\*](https://developer.android.com/studio/write/lint.html) | ✖️ | ✖️ | ✖️ _\* Not supported [yet](https://github.com/novoda/gradle-static-analysis-plugin/issues?q=is%3Aopen+is%3Aissue+label%3A%22new+tool%22)_ From dfc590a4b1b1a6bcd8da6764795694519dfc5465 Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Tue, 30 Jan 2018 15:03:30 +0000 Subject: [PATCH 60/81] Reorganise advanced-usage contents --- docs/advanced-usage.md | 83 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/docs/advanced-usage.md b/docs/advanced-usage.md index e69de29..546fa89 100644 --- a/docs/advanced-usage.md +++ b/docs/advanced-usage.md @@ -0,0 +1,83 @@ +# Advanced configuration + +## Configurable thresholds +Users can define maximum amount of warnings and errors tolerated in a build via the Gradle configuration: + +```gradle +staticAnalysis { + penalty { + maxErrors = 10 + maxWarnings = 10 + } +} +``` + +Violations are then collected while running all the static analysis tools enabled in the project and split between errors and warnings. +Only in the end they are cumulatively evaluated against the thresholds provided in the configuration to decide whether the build should +fail or not. + +## Improve the report with a base URL +Build logs will show an overall report of how many violations have been found during the analysis and the links to +the relevant HTML reports, for instance: + +``` + > PMD rule violations were found (2 errors, 2 warnings). See the reports at: + - file:///foo/project/build/reports/pmd/main.html + - file:///foo/project/build/reports/pmd/main2.html + - file:///foo/project/build/reports/pmd/main3.html + - file:///foo/project/build/reports/pmd/main4.html +``` + +It's possible to specify a custom renderer for the report urls in the logs via the `logs` extension. This can be useful in CI +environments, where the local paths are not reachable directly. For instance the snippet below will replace the base URL with +one mapping to an hypothetical Jenkins workspace: + +```gradle +staticAnalysis { + ... + logs { + reportBaseUrl "http://ci.mycompany.com/job/myproject/ws/app/build/reports" + } +} +``` + +This way, in the CI logs you will see the report URLs printed as: + +``` +> Checkstyle rule violations were found (0 errors, 1 warnings). See the reports at: +- http://ci.mycompany.com/job/myproject/ws/app/build/reports/checkstyle/main.html +``` + +And that will make them easier to follow them to the respective reports. More info on the topic can be found in the +[`LogsExtension`](blob/master/plugin/src/main/groovy/com/novoda/staticanalysis/LogsExtension.groovy) +Groovydocs. + +## Support for `exclude` filters +You can specify custom patterns to exclude specific files from the static analysis. All you have to do is to specify `exclude` +in the configuration of your tool of choice: + +```gradle +staticAnalysis { + findbugs { + exclude '**/*Test.java' // file pattern + exclude project.fileTree('src/test/java') // entire folder + exclude project.file('src/main/java/foo/bar/Constants.java') // specific file + exclude project.sourceSets.main.java.srcDirs // entire source set + } +} +``` + +## Support for Android variants +Sometimes using `exclude` filters could be not enough. When using the plugin in an Android project you may want to consider +only one specific variant as part of the analysis. The plugin provides a way of defining which Android variants should be included +via the `includeVariants` method added to each tool extension. E.g., + +```gradle +staticAnalysis { + findbugs { + includeVariants { variant -> + variant.name.equals('debug') // only the debug variant + } + } +} +``` \ No newline at end of file From 893095289cbcd6ec734f38c90160510ad20bde8c Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Tue, 30 Jan 2018 15:28:48 +0000 Subject: [PATCH 61/81] Add TOC to advanced-usage --- docs/advanced-usage.md | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/docs/advanced-usage.md b/docs/advanced-usage.md index 546fa89..67f4584 100644 --- a/docs/advanced-usage.md +++ b/docs/advanced-usage.md @@ -1,6 +1,17 @@ # Advanced configuration -## Configurable thresholds +The plugin supports a number of advanced behaviours. For example, it can be used to set a baseline of warnings and errors below which +the build will not fail, which is very useful for legacy projects. + +## Table of contents + * [Configurable failure thresholds](#configurable-failure-thresholds) + * [Improve the report with a base URL](#improve-the-report-with-a-base-URL) + * [Add exclusions with `exclude` filters](#add-exclusions-with-exclude-filters) + * [Add exclusions with Android build variants](#add-exclusions-with-Android-build-variants) + +--- + +## Configurable failure thresholds Users can define maximum amount of warnings and errors tolerated in a build via the Gradle configuration: ```gradle @@ -52,7 +63,7 @@ And that will make them easier to follow them to the respective reports. More in [`LogsExtension`](blob/master/plugin/src/main/groovy/com/novoda/staticanalysis/LogsExtension.groovy) Groovydocs. -## Support for `exclude` filters +## Add exclusions with `exclude` filters You can specify custom patterns to exclude specific files from the static analysis. All you have to do is to specify `exclude` in the configuration of your tool of choice: @@ -67,7 +78,7 @@ staticAnalysis { } ``` -## Support for Android variants +## Add exclusions with Android build variants Sometimes using `exclude` filters could be not enough. When using the plugin in an Android project you may want to consider only one specific variant as part of the analysis. The plugin provides a way of defining which Android variants should be included via the `includeVariants` method added to each tool extension. E.g., From 691c20dde141564a6edaefb009d300a5af3ef405 Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Tue, 30 Jan 2018 15:28:59 +0000 Subject: [PATCH 62/81] Create supported-tools --- docs/supported-tools.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 docs/supported-tools.md diff --git a/docs/supported-tools.md b/docs/supported-tools.md new file mode 100644 index 0000000..75029eb --- /dev/null +++ b/docs/supported-tools.md @@ -0,0 +1,12 @@ +# Supported tools + +Tool | Java | Android
(Java) | Kotlin | Android
(Kotlin) +---- | -------- | -------- | ----- | ----- +[`Checkstyle`](https://checkstyle.sourceforge.net) | :white_check_mark: | :white_check_mark: | — | — +[`PMD`](https://pmd.github.io) | :white_check_mark: | :white_check_mark: | — | — +[`FindBugs`](http://findbugs.sourceforge.net/) | :white_check_mark: | :white_check_mark: | — | — +[`Detekt`](https://github.com/arturbosch/detekt) | — | — | :white_check_mark: | :white_check_mark: +[`KtLint`\*](https://github.com/shyiko/ktlint) | — | — | ✖️ | ✖️ +[`Android Lint`\*](https://developer.android.com/studio/write/lint.html) | ✖️ | ✖️ | ✖️| ✖️ + +_\* Not supported [yet](https://github.com/novoda/gradle-static-analysis-plugin/issues?q=is%3Aopen+is%3Aissue+label%3A%22new+tool%22)_ \ No newline at end of file From accf4fcad270547af0af4b0aa4ba79acc2063c9c Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Tue, 30 Jan 2018 15:29:26 +0000 Subject: [PATCH 63/81] Simplify README, point to pages in docs/ --- README.md | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 70e2ac6..75c9769 100644 --- a/README.md +++ b/README.md @@ -14,18 +14,21 @@ The `gradle-static-analysis-plugin` aims to provide: - convenient way of sharing same setup across different projects - healthy, versionable and configurable defaults -### Suported static analysis tools +### Supported static analysis tools +The plugin supports various static analysis tools for Java, Kotlin and Android projects: -Tool | Java | Android (Java) | Kotlin and
Android (Kotlin) ----- | -------- | -------- | ----- -[`Checkstyle`](https://checkstyle.sourceforge.net) | :white_check_mark: | :white_check_mark: | — -[`PMD`](https://pmd.github.io) | :white_check_mark: | :white_check_mark: | — -[`FindBugs`](http://findbugs.sourceforge.net/) | :white_check_mark: | :white_check_mark: | — -[`Detekt`](https://github.com/arturbosch/detekt) | — | — | :white_check_mark: -[`KtLint`\*](https://github.com/shyiko/ktlint) | — | — | ✖️ -[`Android Lint`\*](https://developer.android.com/studio/write/lint.html) | ✖️ | ✖️ | ✖️ + * [`Checkstyle`](https://checkstyle.sourceforge.net) + * [`PMD`](https://pmd.github.io) + * [`FindBugs`](http://findbugs.sourceforge.net/) + * [`Detekt`](https://github.com/arturbosch/detekt) -_\* Not supported [yet](https://github.com/novoda/gradle-static-analysis-plugin/issues?q=is%3Aopen+is%3Aissue+label%3A%22new+tool%22)_ +Support for additional tools is planned but not available yet: + + * [`KtLint`](https://github.com/shyiko/ktlint) + * [`Android Lint`](https://developer.android.com/studio/write/lint.html) + +Please note that the tools availability depends on the project the plugin is applied to. For more details please refer to the +[supported tools](docs/supported-tools.md) page. ### Out-of-the-box support for Android and Kotlin projects Android projects use a Gradle model that is not compatible with the Java one, supported by the built-in static analysis tools plugins. @@ -73,6 +76,8 @@ staticAnalysis { } ``` +For more advanced configurations, please refer to the [advanced usage](docs/advanced-usage.md) page. + ## Roadmap The plugin is under active development and to be considered in **beta stage**. It is routinely used by many Novoda projects and by other external projects with no known critical issues. The API is supposed to be relatively stable, but there still may be From 7c43d5505515ad2948c221e5886fa870d87eb112 Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Tue, 30 Jan 2018 15:30:26 +0000 Subject: [PATCH 64/81] Add missing whiteline at EOF in NOTICE --- NOTICE | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NOTICE b/NOTICE index cca3f3f..a908abf 100644 --- a/NOTICE +++ b/NOTICE @@ -10,4 +10,4 @@ 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. \ No newline at end of file +limitations under the License. From d4c6ef0c7bb147e9cc53e4163ea7793f6dcc35ba Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Tue, 30 Jan 2018 15:32:07 +0000 Subject: [PATCH 65/81] Fix Android Lint support matrix --- docs/supported-tools.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/supported-tools.md b/docs/supported-tools.md index 75029eb..b063844 100644 --- a/docs/supported-tools.md +++ b/docs/supported-tools.md @@ -7,6 +7,6 @@ Tool | Java | Android
(Java) | Kotlin | Android
(Kotlin) [`FindBugs`](http://findbugs.sourceforge.net/) | :white_check_mark: | :white_check_mark: | — | — [`Detekt`](https://github.com/arturbosch/detekt) | — | — | :white_check_mark: | :white_check_mark: [`KtLint`\*](https://github.com/shyiko/ktlint) | — | — | ✖️ | ✖️ -[`Android Lint`\*](https://developer.android.com/studio/write/lint.html) | ✖️ | ✖️ | ✖️| ✖️ +[`Android Lint`\*](https://developer.android.com/studio/write/lint.html) | — | ✖️ | — | ✖️ _\* Not supported [yet](https://github.com/novoda/gradle-static-analysis-plugin/issues?q=is%3Aopen+is%3Aissue+label%3A%22new+tool%22)_ \ No newline at end of file From ce97e867e7b8f99aa11a21efe43836cb566dfa10 Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Tue, 30 Jan 2018 17:46:58 +0000 Subject: [PATCH 66/81] Add documentation for the Checkstyle plugin --- docs/supported-tools.md | 81 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/docs/supported-tools.md b/docs/supported-tools.md index b063844..f7c4f79 100644 --- a/docs/supported-tools.md +++ b/docs/supported-tools.md @@ -1,5 +1,8 @@ # Supported tools +The plugin supports several static analysis tools. The availability of each tool depends on the project the plugin is applied to. +Some tools only support Java code, some only Kotlin code, and some only work on Android projects. To be precise: + Tool | Java | Android
(Java) | Kotlin | Android
(Kotlin) ---- | -------- | -------- | ----- | ----- [`Checkstyle`](https://checkstyle.sourceforge.net) | :white_check_mark: | :white_check_mark: | — | — @@ -9,4 +12,80 @@ Tool | Java | Android
(Java) | Kotlin | Android
(Kotlin) [`KtLint`\*](https://github.com/shyiko/ktlint) | — | — | ✖️ | ✖️ [`Android Lint`\*](https://developer.android.com/studio/write/lint.html) | — | ✖️ | — | ✖️ -_\* Not supported [yet](https://github.com/novoda/gradle-static-analysis-plugin/issues?q=is%3Aopen+is%3Aissue+label%3A%22new+tool%22)_ \ No newline at end of file +_\* Not supported [yet](https://github.com/novoda/gradle-static-analysis-plugin/issues?q=is%3Aopen+is%3Aissue+label%3A%22new+tool%22)_ + +For additional informations and tips on how to obtain advanced behaviours with the plugin and its tools, please refer to the +[advanced usage](advanced-usage.md) page. + +## Table of contents + * [Checkstyle](#checkstyle) + * [Checkstyle in mixed-language projects](#checkstyle-in-mixed-language-projects) + * [PMD](#pmd) _TODO_ + * [Findbugs](#findbugs) _TODO_ + * [Detekt](#detekt) _TODO_ + * KtLint _COMING SOON_ + * Android Lint _COMING SOON_ + * [Example configurations](#example-configurations) + +--- + +## Checkstyle +[Checkstyle](http://checkstyle.sourceforge.net/) is a code style static analysis tool for Java. It is supported for both pure Java and Java Android projects, +but it does not support Kotlin nor Kotlin Android projects. It then only makes sense to have Checkstyle enabled if you have Java code in your project. + +Enabling and configuring Checkstyle for a project is done through the `checkstyle` closure: + +```gradle +checkstyle { + toolVersion // A string, as per http://checkstyle.sourceforge.net/releasenotes.html, e.g., '8.8' + exclude // A fileTree, such as project.fileTree('src/test/java') to exclude Java unit tests + configFile // A string pointing to a Checkstyle config file, e.g., 'config/checkstyle-modules.xml' + includeVariants { variant -> ... } // A closure to determine which variants (for Android) to include +} +``` + +For more informations about Checkstyle rules, refer to the [official website](http://checkstyle.sourceforge.net/checks.html). + +### Checkstyle in mixed-language projects +If your project mixes Java and Kotlin code, you most likely want to have an exclusion in place for all `*.kt` files. You can do so by adding a suppressions file: + +`checkstyle-suppressions.xml` +```xml + + + + + + + + + + +``` + +You then need to reference this file from the Checkstyle configuration file: + +`checkstyle-modules.xml` +```xml + + + + ... + + + + ... + +``` + +(assuming you're using the Novoda scaffolding system, see [Example configurations](#example-configurations) for more details) + +## Example configurations +If you want, you can use the Novoda [`team-props` scaffolding system](https://github.com/novoda/novoda/tree/master/team-props) as a starting point for setting +up your project. The repository contains a good example of [configuration](https://github.com/novoda/novoda/blob/master/team-props/static-analysis.gradle) for +the plugin, and [rulesets](https://github.com/novoda/novoda/tree/master/team-props/static-analysis) for all supported tools. + From 1b327380eb2fc87e800041caf3d4d876efc565c2 Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Tue, 30 Jan 2018 18:00:36 +0000 Subject: [PATCH 67/81] Add instructions for enabling/disabling tools --- docs/supported-tools.md | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/docs/supported-tools.md b/docs/supported-tools.md index f7c4f79..1907cfc 100644 --- a/docs/supported-tools.md +++ b/docs/supported-tools.md @@ -18,6 +18,7 @@ For additional informations and tips on how to obtain advanced behaviours with t [advanced usage](advanced-usage.md) page. ## Table of contents + * [Enable and disable tools](#enable-and-disable-tools) * [Checkstyle](#checkstyle) * [Checkstyle in mixed-language projects](#checkstyle-in-mixed-language-projects) * [PMD](#pmd) _TODO_ @@ -29,9 +30,36 @@ For additional informations and tips on how to obtain advanced behaviours with t --- +## Enable and disable tools +In order to enable a tool, you just need to add it to the `staticAnalysis` closure. To enable all supported tools with their default configurations: + +```gradle +staticAnalysis { + penalty { + // ... (optional) + } + + checkstyle {} + pmd {} + findbugs {} + detekt {} +} +``` + +To disable a tool, simply omit its closure from `staticAnalysis`. This means that, for example, this will not run any tools: + +```gradle +staticAnalysis { + penalty { + // ... + } +} +``` + ## Checkstyle [Checkstyle](http://checkstyle.sourceforge.net/) is a code style static analysis tool for Java. It is supported for both pure Java and Java Android projects, -but it does not support Kotlin nor Kotlin Android projects. It then only makes sense to have Checkstyle enabled if you have Java code in your project. +but it does not support Kotlin nor Kotlin Android projects. It then only makes sense to have Checkstyle enabled if you have Java code in your project. The +plugin only runs Checkstyle on projects that contain the Java or the Android plugin. Enabling and configuring Checkstyle for a project is done through the `checkstyle` closure: @@ -84,6 +112,11 @@ You then need to reference this file from the Checkstyle configuration file: (assuming you're using the Novoda scaffolding system, see [Example configurations](#example-configurations) for more details) +## PMD +[PMD](https://pmd.github.io/) is an extensible cross-language static code analyser. It supports Java and [several other languages](https://pmd.github.io/#about), +but not Kotlin. It can be used in both pure Java, and Android Java projects. It then only makes sense to have PMD enabled if you have Java code in your project. +The plugin only runs PMD on projects that contain the Java or the Android plugin. + ## Example configurations If you want, you can use the Novoda [`team-props` scaffolding system](https://github.com/novoda/novoda/tree/master/team-props) as a starting point for setting up your project. The repository contains a good example of [configuration](https://github.com/novoda/novoda/blob/master/team-props/static-analysis.gradle) for From e0e5ff41fba5aa2abaa6dd80916e5caa86afda8b Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Tue, 30 Jan 2018 18:03:04 +0000 Subject: [PATCH 68/81] Use defaults in config in the README file --- README.md | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 75c9769..69e47aa 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,5 @@ # Gradle static analysis plugin -[![](https://ci.novoda.com/buildStatus/icon?job=gradle-static-analysis-plugin)](https://ci.novoda.com/job/gradle-static-analysis-plugin/lastSuccessfulBuild) [![](https://img.shields.io/badge/License-Apache%202.0-lightgrey.svg)](LICENSE.txt) [![Bintray](https://api.bintray.com/packages/novoda/maven/gradle-static-analysis-plugin/images/download.svg) ](https://bintray.com/novoda/maven/gradle-static-analysis-plugin/_latestVersion) +[![](https://ci.novoda.com/buildStatus/icon?job=gradle-static-analysis-plugin)](https://ci.novoda.com/job/gradle-static-analysis-plugin/lastSuccessfulBuild) [![](https://img.shields.io/badge/License-Apache%202.0-lightgrey.svg)](LICENSE.txt) [![Bintray](https://api.bintray.com/packages/novoda/maven/gradle-static-analysis-plugin/images/download.svg)](https://bintray.com/novoda/maven/gradle-static-analysis-plugin/_latestVersion) A Gradle plugin to easily apply the same setup of static analysis tools across different Android or Java projects. @@ -62,21 +62,17 @@ A typical configuration for the plugin will look like: staticAnalysis { penalty { maxErrors = 0 - maxWarnings = 100 - } - checkstyle { - configFile project.file('path/to/modules.xml') - } - pmd { - ruleSetFiles = project.files('path/to/rules.xml') - } - findbugs { - // ... + maxWarnings = 0 } + checkstyle { } + pmd { } + findbugs { } + detekt { } } ``` -For more advanced configurations, please refer to the [advanced usage](docs/advanced-usage.md) page. +This will enable all the tools with their default settings. For more advanced configurations, please refer to the +[advanced usage](docs/advanced-usage.md) and to the [supported tools](docs/supported-tools.md) pages. ## Roadmap The plugin is under active development and to be considered in **beta stage**. It is routinely used by many Novoda projects and From 40cba312778229515a4c43aaa4a6f72726cac832 Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Tue, 30 Jan 2018 18:34:39 +0000 Subject: [PATCH 69/81] Tweak Detekt missing exception message --- .../staticanalysis/internal/detekt/DetektConfigurator.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index 30f6fd1..7963a6e 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -11,7 +11,7 @@ import org.gradle.api.Task class DetektConfigurator implements Configurator { private static final String DETEKT_PLUGIN = 'io.gitlab.arturbosch.detekt' - private static final String DETEKT_NOT_APPLIED = 'The detekt plugin is configured but not applied. Please apply the plugin in your build script For more information see https://github.com/arturbosch/detekt.' + private static final String DETEKT_NOT_APPLIED = 'The detekt plugin is configured but not applied. Please apply the plugin in your build script.\nFor more informations see https://github.com/arturbosch/detekt.' private final Project project private final Violations violations From 4696307a7779ece72fcb9241d55dac2dc6551784 Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Tue, 30 Jan 2018 18:52:30 +0000 Subject: [PATCH 70/81] Update Missing Detekt message in tests --- plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy index a5a4cdd..45f1684 100644 --- a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy @@ -12,12 +12,14 @@ import javax.annotation.Nullable import static com.novoda.test.TestProject.Result.Logs; class LogsSubject extends Subject { + private static final String VIOLATIONS_LIMIT_EXCEEDED = "Violations limit exceeded" - private static final String DETEKT_NOT_APPLIED = "The detekt plugin is configured but not applied. Please apply the plugin in your build script For more information see https://github.com/arturbosch/detekt." + private static final String DETEKT_NOT_APPLIED = "The detekt plugin is configured but not applied. Please apply the plugin in your build script.\nFor more informations see https://github.com/arturbosch/detekt." private static final String CHECKSTYLE_VIOLATIONS_FOUND = "Checkstyle violations found" private static final String PMD_VIOLATIONS_FOUND = "PMD violations found" private static final String FINDBUGS_VIOLATIONS_FOUND = "Findbugs violations found" private static final String DETEKT_VIOLATIONS_FOUND = "Detekt violations found" + private static final SubjectFactory FACTORY = new SubjectFactory() { @Override LogsSubject getSubject(FailureStrategy failureStrategy, Logs logs) { From 0577d74e45cb0cc27288dfa04ba150956499dd41 Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Tue, 30 Jan 2018 18:56:51 +0000 Subject: [PATCH 71/81] Add documentation for all tools to supported-tools --- docs/supported-tools.md | 161 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 154 insertions(+), 7 deletions(-) diff --git a/docs/supported-tools.md b/docs/supported-tools.md index 1907cfc..34d95db 100644 --- a/docs/supported-tools.md +++ b/docs/supported-tools.md @@ -20,12 +20,20 @@ For additional informations and tips on how to obtain advanced behaviours with t ## Table of contents * [Enable and disable tools](#enable-and-disable-tools) * [Checkstyle](#checkstyle) + * [Configure Checkstyle](#configure-checkstyle) * [Checkstyle in mixed-language projects](#checkstyle-in-mixed-language-projects) - * [PMD](#pmd) _TODO_ - * [Findbugs](#findbugs) _TODO_ - * [Detekt](#detekt) _TODO_ - * KtLint _COMING SOON_ - * Android Lint _COMING SOON_ + * [PMD](#pmd) + * [Configure PMD](#configure-pmd) + * [PMD in mixed-language projects](#pmd-in-mixed-language-projects) + * [Findbugs](#findbugs) + * [Configure Findbugs](#configure-findbugs) + * [Findbugs in mixed-language projects](#findbugs-in-mixed-language-projects) + * [Detekt](#detekt) + * [IMPORTANT: setup Detekt](#important-setup-detekt) + * [Configure Detekt](#configure-detekt) + * [Detekt in mixed-language projects](#detekt-in-mixed-language-projects) + * KtLint — _COMING SOON_ + * Android Lint — _COMING SOON_ * [Example configurations](#example-configurations) --- @@ -61,21 +69,25 @@ staticAnalysis { but it does not support Kotlin nor Kotlin Android projects. It then only makes sense to have Checkstyle enabled if you have Java code in your project. The plugin only runs Checkstyle on projects that contain the Java or the Android plugin. +### Configure Checkstyle Enabling and configuring Checkstyle for a project is done through the `checkstyle` closure: ```gradle checkstyle { toolVersion // A string, as per http://checkstyle.sourceforge.net/releasenotes.html, e.g., '8.8' exclude // A fileTree, such as project.fileTree('src/test/java') to exclude Java unit tests - configFile // A string pointing to a Checkstyle config file, e.g., 'config/checkstyle-modules.xml' + configFile // A file containing the Checkstyle config, e.g., teamPropsFile('static-analysis/checkstyle-modules.xml') includeVariants { variant -> ... } // A closure to determine which variants (for Android) to include } ``` +You can have multiple `exclude` statements. + For more informations about Checkstyle rules, refer to the [official website](http://checkstyle.sourceforge.net/checks.html). ### Checkstyle in mixed-language projects -If your project mixes Java and Kotlin code, you most likely want to have an exclusion in place for all `*.kt` files. You can do so by adding a suppressions file: +If your project mixes Java and Kotlin code, you most likely want to have an exclusion in place for all `*.kt` files. You can use the `exclude` +in the configuration closure, or you can do so by adding a suppressions file: `checkstyle-suppressions.xml` ```xml @@ -117,6 +129,141 @@ You then need to reference this file from the Checkstyle configuration file: but not Kotlin. It can be used in both pure Java, and Android Java projects. It then only makes sense to have PMD enabled if you have Java code in your project. The plugin only runs PMD on projects that contain the Java or the Android plugin. +### Configure PMD +Enabling and configuring PMD for a project is done through the `pmd` closure: + +```gradle +pmd { + toolVersion // A string, as per https://pmd.github.io/pmd-6.0.1/pmd_release_notes.html, e.g., '6.0.1' + exclude // A fileTree, such as project.fileTree('src/test/java') to exclude Java unit tests + ruleSetFiles // A set of files containing PMD rulesets, e.g., rootProject.files('team-props/static-analysis/pmd-rules.xml') + ruleSets = [] // Note: this is a workaround to make the s in pmd-rules.xml actually work + includeVariants { variant -> ... } // A closure to determine which variants (for Android) to include +} +``` + +You can have multiple `exclude` statements. + +For more informations about PMD Java rules, refer to the [official website](https://pmd.github.io/pmd-6.0.1/pmd_rules_java.html). + +### PMD in mixed-language projects +If your project mixes Java and Kotlin code, you most likely want to have an exclusion in place for all `*.kt` files. You can use the `exclude` +in the configuration closure, or you can do so by adding a suppressions file: + +`pmd-rules.xml` +```xml + + + + ... + + + .*\.kt + + + ... + + +``` + +## Findbugs +[Findbugs](http://findbugs.sourceforge.net/) is a static analysis tool that looks for potential bugs in Java code. It does not support Kotlin. +It can be used in both pure Java, and Android Java projects. It then only makes sense to have Findbugs enabled if you have Java code in your project. +The plugin only runs Findbugs on projects that contain the Java or the Android plugin. + +### Configure Findbugs +Enabling and configuring Findbugs for a project is done through the `findbugs` closure: + +```gradle +findbugs { + toolVersion // A string, most likely '3.0.1' — the latest Findbugs release (for a long time) + exclude // A fileTree, such as project.fileTree('src/test/java') to exclude Java unit tests + excludeFilter // A file containing the Findbugs exclusions, e.g., teamPropsFile('static-analysis/findbugs-excludes.xml') + includeVariants { variant -> ... } // A closure to determine which variants (for Android) to include +} +``` + +You can have multiple `exclude` statements. + +For more informations about Findbugs rules, refer to the [official website](http://findbugs.sourceforge.net/bugDescriptions.html). + +### Findbugs in mixed-language projects +If your project mixes Java and Kotlin code, you most likely want to have an exclusion in place for all `*.kt` files. You can use the `exclude` +in the configuration closure, or you can do so by adding a suppression to the filter file: + +`findbugs-excludes.xml` +```xml + + + ... + + + + + + ... + + +``` + +## Detekt +[Detekt](https://github.com/arturbosch/detekt) is a static analysis tool that looks for potential bugs and style violations in Kotlin code. It +does not support Java. It can be used in both pure Kotlin, and Android Kotlin projects. It then only makes sense to have Detekt enabled if you +have Kotlin code in your project. The plugin only runs Detekt on projects that contain the Kotlin or the Kotlin-Android plugin. + +### IMPORTANT: setup Detekt +Unlike the other tools, the plugin **won't automatically add Detekt** to your project. If you forget to do it, the plugin will fail the build +with an error like: + +``` +The detekt plugin is configured but not applied. Please apply the plugin in your build script. +For more information see https://github.com/arturbosch/detekt. +``` + +In order to use Detekt in your project, you need to: + + 1. Add this statement to your root `build.gradle` project (change the version according to your needs): + ```gradle + plugins { + id 'io.gitlab.arturbosch.detekt' version '1.0.0.RC6-2' + // ... + } + ``` + 2. Add this statement to each Kotlin project's `build.gradle`s: + ```gradle + plugins { + id 'io.gitlab.arturbosch.detekt' + // ... + } + ``` + +### Configure Detekt +Enabling and configuring Detekt for a project is done through the `detekt` closure. The closure behaves exactly like the +[standard Detekt plugin](https://github.com/arturbosch/detekt#using-the-detekt-gradle-plugin) does in Gradle, which is to say, quite differently +from how the other tools' configurations closures work. For example: + +```gradle +detekt { + profile('main') { + input = "$projectDir/src/main/java" + config = teamPropsFile('static-analysis/detekt-config.yml') + filters = '.*test.*,.*/resources/.*,.*/tmp/.*' + output = "$projectDir/build/reports/detekt" + } +} +``` + +You need to provide **at a minimum** the `config` and `output` values. + +For more informations about Detekt rules, refer to the [official website](https://github.com/arturbosch/detekt/tree/master/detekt-generator/documentation). + +### Detekt in mixed-language projects +If your project mixes Java and Kotlin code, you don't need to have an exclusion in place for all `*.java` files. Detekt itself only looks for +`*.kt` files, so no further configuration is required. + ## Example configurations If you want, you can use the Novoda [`team-props` scaffolding system](https://github.com/novoda/novoda/tree/master/team-props) as a starting point for setting up your project. The repository contains a good example of [configuration](https://github.com/novoda/novoda/blob/master/team-props/static-analysis.gradle) for From 9cc578057eb2e41c89895ddbdeb4d7d9edeba0b6 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane Date: Wed, 31 Jan 2018 09:49:27 +0100 Subject: [PATCH 72/81] Fix typo --- .../staticanalysis/internal/detekt/DetektConfigurator.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index 7963a6e..43f4af1 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -11,7 +11,7 @@ import org.gradle.api.Task class DetektConfigurator implements Configurator { private static final String DETEKT_PLUGIN = 'io.gitlab.arturbosch.detekt' - private static final String DETEKT_NOT_APPLIED = 'The detekt plugin is configured but not applied. Please apply the plugin in your build script.\nFor more informations see https://github.com/arturbosch/detekt.' + private static final String DETEKT_NOT_APPLIED = 'The detekt plugin is configured but not applied. Please apply the plugin in your build script.\nFor more information see https://github.com/arturbosch/detekt.' private final Project project private final Violations violations From fb10e7f7fcc6d0ad7ec7fdef9475af13543cb670 Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Wed, 31 Jan 2018 09:07:13 +0000 Subject: [PATCH 73/81] Fix grammar in error message --- .../staticanalysis/internal/detekt/DetektConfigurator.groovy | 2 +- plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index 43f4af1..dfa5615 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -11,7 +11,7 @@ import org.gradle.api.Task class DetektConfigurator implements Configurator { private static final String DETEKT_PLUGIN = 'io.gitlab.arturbosch.detekt' - private static final String DETEKT_NOT_APPLIED = 'The detekt plugin is configured but not applied. Please apply the plugin in your build script.\nFor more information see https://github.com/arturbosch/detekt.' + private static final String DETEKT_NOT_APPLIED = 'The Detekt plugin is configured but not applied. Please apply the plugin in your build script.\nFor more information see https://github.com/arturbosch/detekt.' private final Project project private final Violations violations diff --git a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy index 45f1684..29d7856 100644 --- a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy @@ -14,7 +14,7 @@ import static com.novoda.test.TestProject.Result.Logs; class LogsSubject extends Subject { private static final String VIOLATIONS_LIMIT_EXCEEDED = "Violations limit exceeded" - private static final String DETEKT_NOT_APPLIED = "The detekt plugin is configured but not applied. Please apply the plugin in your build script.\nFor more informations see https://github.com/arturbosch/detekt." + private static final String DETEKT_NOT_APPLIED = "The Detekt plugin is configured but not applied. Please apply the plugin in your build script." private static final String CHECKSTYLE_VIOLATIONS_FOUND = "Checkstyle violations found" private static final String PMD_VIOLATIONS_FOUND = "PMD violations found" private static final String FINDBUGS_VIOLATIONS_FOUND = "Findbugs violations found" From f0d2803fc8f361db80d1e3b0ccec2f5c280a811f Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Wed, 31 Jan 2018 09:30:53 +0000 Subject: [PATCH 74/81] Cleanup DetektConfigurator --- .../staticanalysis/internal/detekt/DetektConfigurator.groovy | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index dfa5615..8e7ef69 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -32,8 +32,7 @@ class DetektConfigurator implements Configurator { @Override void execute() { - project.extensions.findByType(StaticAnalysisExtension).ext."detekt" = { Closure config -> - + project.extensions.findByType(StaticAnalysisExtension).ext.'detekt' = { Closure config -> if (!isKotlinProject(project)) { return } @@ -66,7 +65,7 @@ class DetektConfigurator implements Configurator { private CollectDetektViolationsTask createCollectViolationsTask(Violations violations) { def outputFolder = project.file(project.extensions.findByName('detekt').systemOrDefaultProfile().output) - project.tasks.create("collectDetektViolations", CollectDetektViolationsTask) { collectViolations -> + project.tasks.create('collectDetektViolations', CollectDetektViolationsTask) { collectViolations -> collectViolations.xmlReportFile = new File(outputFolder, 'detekt-checkstyle.xml') collectViolations.htmlReportFile = new File(outputFolder, 'detekt-report.html') collectViolations.violations = violations From 2a024202e2a73f835b870b0b7242b9c78b9fd486 Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Wed, 31 Jan 2018 09:37:24 +0000 Subject: [PATCH 75/81] Cleanup LogsSubject --- .../test/groovy/com/novoda/test/LogsSubject.groovy | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy index 29d7856..ed089b9 100644 --- a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy @@ -13,12 +13,12 @@ import static com.novoda.test.TestProject.Result.Logs; class LogsSubject extends Subject { - private static final String VIOLATIONS_LIMIT_EXCEEDED = "Violations limit exceeded" - private static final String DETEKT_NOT_APPLIED = "The Detekt plugin is configured but not applied. Please apply the plugin in your build script." - private static final String CHECKSTYLE_VIOLATIONS_FOUND = "Checkstyle violations found" - private static final String PMD_VIOLATIONS_FOUND = "PMD violations found" - private static final String FINDBUGS_VIOLATIONS_FOUND = "Findbugs violations found" - private static final String DETEKT_VIOLATIONS_FOUND = "Detekt violations found" + private static final String VIOLATIONS_LIMIT_EXCEEDED = 'Violations limit exceeded' + private static final String DETEKT_NOT_APPLIED = 'The Detekt plugin is configured but not applied. Please apply the plugin in your build script.' + private static final String CHECKSTYLE_VIOLATIONS_FOUND = 'Checkstyle violations found' + private static final String PMD_VIOLATIONS_FOUND = 'PMD violations found' + private static final String FINDBUGS_VIOLATIONS_FOUND = 'Findbugs violations found' + private static final String DETEKT_VIOLATIONS_FOUND = 'Detekt violations found' private static final SubjectFactory FACTORY = new SubjectFactory() { @Override From f9b8f07d553469d2540395e3b5a9739abc39ed25 Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Wed, 31 Jan 2018 10:21:53 +0000 Subject: [PATCH 76/81] Shorten a title in README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 69e47aa..9738ccb 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ The `gradle-static-analysis-plugin` aims to provide: - convenient way of sharing same setup across different projects - healthy, versionable and configurable defaults -### Supported static analysis tools +### Supported tools The plugin supports various static analysis tools for Java, Kotlin and Android projects: * [`Checkstyle`](https://checkstyle.sourceforge.net) From cb7a7d4eae55d8d31d7642abf765a49a2c755257 Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Wed, 31 Jan 2018 10:22:31 +0000 Subject: [PATCH 77/81] Remove "Kotlin" from a title in README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 9738ccb..7274762 100644 --- a/README.md +++ b/README.md @@ -30,7 +30,7 @@ Support for additional tools is planned but not available yet: Please note that the tools availability depends on the project the plugin is applied to. For more details please refer to the [supported tools](docs/supported-tools.md) page. -### Out-of-the-box support for Android and Kotlin projects +### Out-of-the-box support for Android projects Android projects use a Gradle model that is not compatible with the Java one, supported by the built-in static analysis tools plugins. Applying `gradle-static-analysis-plugin` to your Android project will make sure all the necessary tasks are created and correctly configured without any additional hassle. From 21430dcd8a758a6a56d1690c4363aaedd7f053ca Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Wed, 31 Jan 2018 10:50:46 +0000 Subject: [PATCH 78/81] Note that excludes don't work for Detekt --- docs/advanced-usage.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/advanced-usage.md b/docs/advanced-usage.md index 67f4584..d62d337 100644 --- a/docs/advanced-usage.md +++ b/docs/advanced-usage.md @@ -78,6 +78,9 @@ staticAnalysis { } ``` +Please note that this is not supported for Detekt. To exclude files in Detekt, please refer to the specific tool documentation +in the [supported tools](supported-tools.md) page. + ## Add exclusions with Android build variants Sometimes using `exclude` filters could be not enough. When using the plugin in an Android project you may want to consider only one specific variant as part of the analysis. The plugin provides a way of defining which Android variants should be included @@ -91,4 +94,6 @@ staticAnalysis { } } } -``` \ No newline at end of file +``` + +Please note that this is not supported for Detekt yet. \ No newline at end of file From a38c70b748b653e46cae764d2025c26faa061f77 Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Wed, 31 Jan 2018 10:51:54 +0000 Subject: [PATCH 79/81] Note not to configure limits or failFast for Detekt --- docs/supported-tools.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/supported-tools.md b/docs/supported-tools.md index 34d95db..abbe2c3 100644 --- a/docs/supported-tools.md +++ b/docs/supported-tools.md @@ -256,7 +256,9 @@ detekt { } ``` -You need to provide **at a minimum** the `config` and `output` values. +You need to provide **at a minimum** the `config` and `output` values. It's important that you do _not_ specify a `warningThreshold` nor a `failThreshold` +in the Detekt configuration file as it will interfere with the functioning of the Static Analysis plugin's threshold counting. For the same reason, make +sure that `failFast` is set to `false` in the Detekt configuration. For more informations about Detekt rules, refer to the [official website](https://github.com/arturbosch/detekt/tree/master/detekt-generator/documentation). From b6e2e1e1cbed38532bb1abb1ed8e409a4380d469 Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Wed, 31 Jan 2018 11:31:38 +0000 Subject: [PATCH 80/81] Explain how to exclude files from Detekt analysis --- docs/advanced-usage.md | 2 +- docs/supported-tools.md | 17 +++++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/docs/advanced-usage.md b/docs/advanced-usage.md index d62d337..d723d37 100644 --- a/docs/advanced-usage.md +++ b/docs/advanced-usage.md @@ -79,7 +79,7 @@ staticAnalysis { ``` Please note that this is not supported for Detekt. To exclude files in Detekt, please refer to the specific tool documentation -in the [supported tools](supported-tools.md) page. +in the [supported tools](supported-tools.md#exclude-files-from-detekt-analysis) page. ## Add exclusions with Android build variants Sometimes using `exclude` filters could be not enough. When using the plugin in an Android project you may want to consider diff --git a/docs/supported-tools.md b/docs/supported-tools.md index abbe2c3..1c42476 100644 --- a/docs/supported-tools.md +++ b/docs/supported-tools.md @@ -31,6 +31,7 @@ For additional informations and tips on how to obtain advanced behaviours with t * [Detekt](#detekt) * [IMPORTANT: setup Detekt](#important-setup-detekt) * [Configure Detekt](#configure-detekt) + * [Exclude files from Detekt analysis](#exclude-files-from-detekt-analysis) * [Detekt in mixed-language projects](#detekt-in-mixed-language-projects) * KtLint — _COMING SOON_ * Android Lint — _COMING SOON_ @@ -248,10 +249,10 @@ from how the other tools' configurations closures work. For example: ```gradle detekt { profile('main') { - input = "$projectDir/src/main/java" - config = teamPropsFile('static-analysis/detekt-config.yml') - filters = '.*test.*,.*/resources/.*,.*/tmp/.*' - output = "$projectDir/build/reports/detekt" + input = // A string pointing to a project's sources. E.g., "$projectDir/src/main/java" + config = // A file containing the Detekt configuration, e.g., teamPropsFile('static-analysis/detekt-config.yml') + filters = // A comma-separated list of regex exclusions, e.g., '.*test.*,.*/resources/.*,.*/tmp/.*' + output = // A string pointing to the output directory for the reports, e.g., "$projectDir/build/reports/detekt" } } ``` @@ -262,6 +263,14 @@ sure that `failFast` is set to `false` in the Detekt configuration. For more informations about Detekt rules, refer to the [official website](https://github.com/arturbosch/detekt/tree/master/detekt-generator/documentation). +### Exclude files from Detekt analysis + +In order to exclude files from Detekt analysis, you have to use the facilities provided by the Detekt plugin in the `detekt` configuration closure. This means, +you have to provide a value to the `filters` property that contains the exclusion pattern(s) you wish Detekt to ignore. + +The `filters` property expects a string consisting in a comma-separated list of regular expression patterns, e.g., `'.*test.*,.*/resources/.*,.*/tmp/.*'` +(which would exclude any path containing the word `test`, or any path containing a directory called `resources` or `tmp`). + ### Detekt in mixed-language projects If your project mixes Java and Kotlin code, you don't need to have an exclusion in place for all `*.java` files. Detekt itself only looks for `*.kt` files, so no further configuration is required. From fadaac3f765705d44d1c12b14ec0d7f8d643688c Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Wed, 31 Jan 2018 11:34:52 +0000 Subject: [PATCH 81/81] Tweak wording for adding Detekt to a project --- docs/supported-tools.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/supported-tools.md b/docs/supported-tools.md index 1c42476..1e0c6f3 100644 --- a/docs/supported-tools.md +++ b/docs/supported-tools.md @@ -217,14 +217,14 @@ have Kotlin code in your project. The plugin only runs Detekt on projects that c ### IMPORTANT: setup Detekt Unlike the other tools, the plugin **won't automatically add Detekt** to your project. If you forget to do it, the plugin will fail the build -with an error like: +with an error. -``` -The detekt plugin is configured but not applied. Please apply the plugin in your build script. -For more information see https://github.com/arturbosch/detekt. -``` +In order to use Detekt, you need to manually add it to **all** your Kotlin projects. You can refer to the +[official documentation](https://github.com/arturbosch/detekt/#gradlegroovy) for further details. Note that you should _not_ add the `detekt` +closure to your `build.gradle`s, unlike what the official documentation says. The `detekt` closure in the `staticAnalysis` configuration gets +applied to all Kotlin modules automatically. -In order to use Detekt in your project, you need to: +In most common cases, adding Detekt to a project boils down to three simple steps: 1. Add this statement to your root `build.gradle` project (change the version according to your needs): ```gradle