Skip to content
This repository has been archived by the owner on Feb 11, 2022. It is now read-only.

Android lint variant filter support #79

Merged
merged 14 commits into from Feb 13, 2018
Merged
Expand Up @@ -15,8 +15,8 @@ class StaticAnalysisPlugin implements Plugin<Project> {

@Override
void apply(Project project) {
StaticAnalysisExtension pluginExtension = project.extensions.create('staticAnalysis', StaticAnalysisExtension, project)
Task evaluateViolations = createEvaluateViolationsTask(project, pluginExtension)
def pluginExtension = project.extensions.create('staticAnalysis', StaticAnalysisExtension, project)
def evaluateViolations = createEvaluateViolationsTask(project, pluginExtension)
createConfigurators(project, pluginExtension, evaluateViolations).each { configurator -> configurator.execute() }
project.afterEvaluate {
project.tasks['check'].dependsOn evaluateViolations
Expand Down
Expand Up @@ -15,14 +15,14 @@ abstract class CodeQualityConfigurator<T extends SourceTask, E extends CodeQuali
protected final Violations violations
protected final Task evaluateViolations
protected final SourceFilter sourceFilter
protected Closure<Boolean> includeVariantsFilter
protected final VariantFilter variantFilter

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 }
this.variantFilter = new VariantFilter(project)
}

@Override
Expand All @@ -32,19 +32,19 @@ abstract class CodeQualityConfigurator<T extends SourceTask, E extends CodeQuali
project.extensions.findByType(extensionClass).with {
defaultConfiguration.execute(it)
ext.exclude = { Object rule -> sourceFilter.exclude(rule) }
ext.includeVariants = { Closure<Boolean> filter -> includeVariantsFilter = filter }
ext.includeVariants = { Closure<Boolean> filter -> variantFilter.includeVariantsFilter = filter }
config.delegate = it
config()
}
project.plugins.withId('com.android.application') {
project.afterEvaluate {
configureAndroidProject(allApplicationVariants.matching { includeVariantsFilter(it) })
configureAndroidProject(variantFilter.filteredApplicationAndTestVariants)
configureToolTasks()
}
}
project.plugins.withId('com.android.library') {
project.afterEvaluate {
configureAndroidProject(allLibraryVariants.matching { includeVariantsFilter(it) })
configureAndroidProject(variantFilter.filteredLibraryAndTestVariants)
configureToolTasks()
}
}
Expand All @@ -57,29 +57,13 @@ abstract class CodeQualityConfigurator<T extends SourceTask, E extends CodeQuali
}
}

protected NamedDomainObjectSet<Object> getAllApplicationVariants() {
getAllVariants(project.android.applicationVariants)
}

protected void configureToolTasks() {
project.tasks.withType(taskClass) { task ->
task.group = 'verification'
configureReportEvaluation(task, violations)
}
}

protected NamedDomainObjectSet<Object> getAllLibraryVariants() {
getAllVariants(project.android.libraryVariants)
}

private NamedDomainObjectSet<Object> getAllVariants(variants1) {
NamedDomainObjectSet<Object> 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() {
Expand Down
@@ -0,0 +1,43 @@
package com.novoda.staticanalysis.internal

import org.gradle.api.DomainObjectSet
import org.gradle.api.NamedDomainObjectSet
import org.gradle.api.Project

final class VariantFilter {
Copy link
Contributor

Choose a reason for hiding this comment

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


private final Project project
Closure<Boolean> includeVariantsFilter

VariantFilter(Project project) {
this.project = project
}

DomainObjectSet<Object> getFilteredApplicationVariants() {
filterVariants(project.android.applicationVariants)
}

NamedDomainObjectSet<Object> getFilteredApplicationAndTestVariants() {
filterVariants(getAllVariants(project.android.applicationVariants))
}

DomainObjectSet<Object> getFilteredLibraryVariants() {
filterVariants(project.android.libraryVariants)
}

NamedDomainObjectSet<Object> getFilteredLibraryAndTestVariants() {
filterVariants(getAllVariants(project.android.libraryVariants))
}

private def filterVariants(variants) {
includeVariantsFilter != null ? variants.matching { includeVariantsFilter(it) } : variants
}

private NamedDomainObjectSet<Object> getAllVariants(variants1) {
NamedDomainObjectSet<Object> variants = project.container(Object)
variants.addAll(variants1)
variants.addAll(project.android.testVariants)
variants.addAll(project.android.unitTestVariants)
return variants
}
}
Expand Up @@ -3,6 +3,8 @@ package com.novoda.staticanalysis.internal.lint
import com.novoda.staticanalysis.StaticAnalysisExtension
import com.novoda.staticanalysis.Violations
import com.novoda.staticanalysis.internal.Configurator
import com.novoda.staticanalysis.internal.VariantFilter
import org.gradle.api.DomainObjectSet
import org.gradle.api.NamedDomainObjectContainer
import org.gradle.api.Project
import org.gradle.api.Task
Expand All @@ -12,6 +14,7 @@ class LintConfigurator implements Configurator {
private final Project project
private final Violations violations
private final Task evaluateViolations
private final VariantFilter variantFilter

static LintConfigurator create(Project project,
NamedDomainObjectContainer<Violations> violationsContainer,
Expand All @@ -24,20 +27,27 @@ class LintConfigurator implements Configurator {
this.project = project
this.violations = violations
this.evaluateViolations = evaluateViolations
this.variantFilter = new VariantFilter(project)
}

@Override
void execute() {
project.extensions.findByType(StaticAnalysisExtension).ext."lintOptions" = { Closure config ->
if (!isAndroidProject(project)) {
return
project.extensions.findByType(StaticAnalysisExtension).ext.lintOptions = { Closure config ->
project.plugins.withId('com.android.application') {
configureLint(config)
configureWithVariants(variantFilter.filteredApplicationVariants)
}
project.plugins.withId('com.android.library') {
configureLint(config)
configureWithVariants(variantFilter.filteredLibraryVariants)
}
configureLint(config)
configureToolTask()
}
}

private void configureLint(Closure config) {
project.android.lintOptions.ext.includeVariants = { Closure<Boolean> filter ->
Copy link
Contributor

Choose a reason for hiding this comment

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

@mr-archano
Could u explain us what exactly happens in this closure?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what you're asking @tobiasheine (as I didn't write this), but I will try to explain what I can see going on here: L54 adds an extension method to lintOptions that takes a filter predicate, used by VariantAware.variantSelector to actually filter in only the variants for which filter is evaluated as true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, he was asking because this is a copy paste from the other ones.
I was also wondering how this works exactly. One question comes to mind, there is no mention of the input of the closure. We expect the input to be a variant but I guess there is no limit? Can it be anything?

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand that we define a extension includeVariants with a Closure<Boolean> and what it is used for.
What is confusing to me is the assignment includeVariantsFilter = filter within the closure or when it is executed. Is it like a callback that is triggered when the user specifies the extension?

variantFilter.includeVariantsFilter = filter
}
project.android.lintOptions(config)
project.android.lintOptions {
xmlReport = true
Expand All @@ -46,32 +56,41 @@ class LintConfigurator implements Configurator {
}
}

private void configureToolTask() {
// evaluate violations after lint
def collectViolations = createCollectViolationsTask(violations)
private void configureWithVariants(DomainObjectSet variants) {
if (variantFilter.includeVariantsFilter != null) {
variants.all {
configureCollectViolationsTask(it.name, "lint-results-${it.name}")
Copy link
Contributor

Choose a reason for hiding this comment

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

me happy! 🐶

}
} else {
configureCollectViolationsTask('lint-results')
}
}

private void configureCollectViolationsTask(String taskSuffix = '', String reportFileName) {
def collectViolations = createCollectViolationsTask(taskSuffix, reportFileName, violations).with {
it.dependsOn project.tasks.findByName("lint${taskSuffix.capitalize()}")
}
evaluateViolations.dependsOn collectViolations
collectViolations.dependsOn project.tasks['lint']
}

private CollectLintViolationsTask createCollectViolationsTask(Violations violations) {
project.tasks.create('collectLintViolations', CollectLintViolationsTask) { collectViolations ->
collectViolations.xmlReportFile = xmlOutputFile
collectViolations.htmlReportFile = new File(defaultOutputFolder, 'lint-results.html')
collectViolations.violations = violations
private CollectLintViolationsTask createCollectViolationsTask(String taskSuffix, String reportFileName, Violations violations) {
project.tasks.create("collectLint${taskSuffix.capitalize()}Violations", CollectLintViolationsTask) { task ->
task.xmlReportFile = xmlOutputFileFor(reportFileName)
task.htmlReportFile = htmlOutputFileFor(reportFileName)
task.violations = violations
}
}

private File getXmlOutputFile() {
project.android.lintOptions.xmlOutput ?: new File(defaultOutputFolder, 'lint-results.xml')
private File xmlOutputFileFor(reportFileName) {
project.android.lintOptions.xmlOutput ?: new File(defaultOutputFolder, "${reportFileName}.xml")
}

private File htmlOutputFileFor(reportFileName) {
project.android.lintOptions.htmlOutput ?: new File(defaultOutputFolder, "${reportFileName}.html")
}

private File getDefaultOutputFolder() {
new File(project.buildDir, 'reports')
}

private static boolean isAndroidProject(final Project project) {
final boolean isAndroidApplication = project.plugins.hasPlugin('com.android.application')
final boolean isAndroidLibrary = project.plugins.hasPlugin('com.android.library')
return isAndroidApplication || isAndroidLibrary
}
}
Expand Up @@ -11,7 +11,7 @@ import static com.google.common.truth.Truth.assertThat
class CollectLintViolationsTaskTest {

@Test
void shouldAddResultsToViolations() throws Exception {
void shouldAddResultsToViolations() {
Project project = ProjectBuilder.builder().build()
def task = project.task('collectLintViolationsTask', type: CollectLintViolationsTask)

Expand Down
@@ -0,0 +1,119 @@
package com.novoda.staticanalysis.internal.lint

import com.google.common.truth.Truth
import com.novoda.test.Fixtures
import com.novoda.test.TestAndroidProject
import com.novoda.test.TestProject
import com.novoda.test.TestProjectRule
import org.junit.Rule
import org.junit.Test

import static com.novoda.test.LogsSubject.assertThat

class LintAndroidVariantIntegrationTest {

@Rule
public final TestProjectRule<TestAndroidProject> projectRule = TestProjectRule.forAndroidProject()

@Test
void shouldFailBuildWhenLintViolationsOverThreshold() {
TestProject.Result result = starterProject()
.withSourceSet('demo', Fixtures.Lint.SOURCES_WITH_ERRORS)
.withToolsConfig(DEFAULT_CONFIG)
.buildAndFail('evaluateViolations')

assertThat(result.logs).containsLimitExceeded(1, 1)
assertThat(result.logs).containsLintViolations(1, 1,
'reports/lint-results.html')
}

@Test
void givenVariantsFilteredShouldFailBuildWithDuplicatedNumbers() {
TestProject.Result result = starterProject()
.withSourceSet('demo', Fixtures.Lint.SOURCES_WITH_ERRORS)
.withToolsConfig(configWithVariants('demoDebug', 'fullRelease'))
.buildAndFail('evaluateViolations')

assertThat(result.logs).containsLimitExceeded(1, 2)
assertThat(result.logs).containsLintViolations(1, 2,
'reports/lint-results-demoDebug.html',
'reports/lint-results-fullRelease.html'
)
}

@Test
void shouldIgnoreErrorsFromInactiveVariant() {
TestProject.Result result = starterProject(maxWarnings: 1)
.withSourceSet('demo', Fixtures.Lint.SOURCES_WITH_ERRORS)
.withToolsConfig(configWithVariants('fullRelease'))
.build('evaluateViolations')

assertThat(result.logs).containsLintViolations(0, 1)
}

@Test
void shouldContainCollectLintTasks() {
TestProject.Result result = starterProject(maxWarnings: 1)
.withToolsConfig(DEFAULT_CONFIG)
.build('evaluateViolations')

Truth.assertThat(result.tasksPaths).containsAllOf(
':lint',
':collectLintViolations',
)
}

@Test
void givenVariantsFilteredShouldContainTasksForIncludedVariantsOnly() {
TestProject.Result result = starterProject(maxWarnings: 1)
.withToolsConfig(configWithVariants('demoDebug'))
.build('evaluateViolations')

Truth.assertThat(result.tasksPaths).containsAllOf(
':lintDemoDebug',
':collectLintDemoDebugViolations')
Truth.assertThat(result.tasksPaths).containsNoneOf(
':lint',
':lintDemoRelease',
':collectLintDemoReleaseViolations',
':lintFullDebug',
':collectLintFullDebugViolations',
':lintFullRelease',
':collectLintFullReleaseViolations')
}

private static final String DEFAULT_CONFIG =
"""
lintOptions {
lintConfig = file("${Fixtures.Lint.RULES}")
}
"""

private static configWithVariants(String... variantNames) {
def commaSeparatedVariants = variantNames.collect { "'$it'" }.join(', ')
"""
lintOptions {
checkReleaseBuilds false
lintConfig = file("${Fixtures.Lint.RULES}")
includeVariants { it.name in [$commaSeparatedVariants] }
}
"""
}

private TestAndroidProject starterProject(def args = [:]) {
projectRule.newProject()
.withSourceSet('main', Fixtures.Lint.SOURCES_WITH_WARNINGS)
.withPenalty("""{
maxErrors = ${args.maxErrors ?: 0}
maxWarnings = ${args.maxWarnings ?: 0}
}""")
.withAdditionalAndroidConfig('''
flavorDimensions 'tier'

productFlavors {
demo { dimension 'tier' }
full { dimension 'tier' }
}
''')
}
}
Expand Up @@ -17,31 +17,31 @@ class LintIntegrationTest {
"""

@Test
void shouldFailBuildWhenLintErrorsOverTheThresholds() throws Exception {
void shouldFailBuildWhenLintErrorsOverTheThresholds() {
def result = createAndroidProjectWith(Fixtures.Lint.SOURCES_WITH_ERRORS, 0, 0)
.buildAndFail('check')

assertThat(result.logs).containsLintViolations(1, 0, 'reports/lint-results.html')
}

@Test
void shouldNotFailBuildWhenLintErrorsWithinTheThresholds() throws Exception {
void shouldNotFailBuildWhenLintErrorsWithinTheThresholds() {
def result = createAndroidProjectWith(Fixtures.Lint.SOURCES_WITH_ERRORS, 0, 1)
.build('check')

assertThat(result.logs).doesNotContainLimitExceeded()
}

@Test
void shouldFailBuildWhenLintWarningsOverTheThresholds() throws Exception {
void shouldFailBuildWhenLintWarningsOverTheThresholds() {
def result = createAndroidProjectWith(Fixtures.Lint.SOURCES_WITH_WARNINGS, 0, 0)
.buildAndFail('check')

assertThat(result.logs).containsLintViolations(0, 1, 'reports/lint-results.html')
}

@Test
void shouldNotFailBuildWhenLintWarningsWithinTheThresholds() throws Exception {
void shouldNotFailBuildWhenLintWarningsWithinTheThresholds() {
def result = createAndroidProjectWith(Fixtures.Lint.SOURCES_WITH_WARNINGS, 1, 0)
.build('check')

Expand Down