From cea0ab94014e0b8a7e4489c6bf53ef4715720bbf Mon Sep 17 00:00:00 2001 From: "mohammad.khaleghi" Date: Tue, 16 Apr 2024 10:56:59 +0800 Subject: [PATCH] adding control over lint exit code for warning and information issues --- rules/android/android_binary.bzl | 2 + rules/android/android_library.bzl | 2 + rules/android/lint/lint_aspect.bzl | 8 + rules/android/lint/lint_sources.bzl | 4 + rules/android/lint/providers.bzl | 2 + rules/kotlin/kotlin.bzl | 2 + .../java/com/grab/lint/LintReportCommand.kt | 17 +- .../main/java/com/grab/lint/LintResults.kt | 13 +- .../java/com/grab/lint/LintResultsTest.kt | 163 +++++++++++++++++- 9 files changed, 210 insertions(+), 3 deletions(-) diff --git a/rules/android/android_binary.bzl b/rules/android/android_binary.bzl index 622beae6..5359af8c 100644 --- a/rules/android/android_binary.bzl +++ b/rules/android/android_binary.bzl @@ -83,6 +83,8 @@ def android_binary( lint_config = lint_options.get("config", None), deps = kotlin_library_deps, lint_checks = lint_options.get("lint_checks", default = []), + fail_on_warning = lint_options.get("fail_on_warning", default = True), + fail_on_information = lint_options.get("fail_on_information", default = True), ) # Build deps diff --git a/rules/android/android_library.bzl b/rules/android/android_library.bzl index 9b965ae8..da0ac8aa 100644 --- a/rules/android/android_library.bzl +++ b/rules/android/android_library.bzl @@ -67,6 +67,8 @@ def android_library( lint_config = lint_options.get("config", None), deps = android_library_deps, lint_checks = lint_options.get("lint_checks", default = []), + fail_on_warning = lint_options.get("fail_on_warning", default = True), + fail_on_information = lint_options.get("fail_on_information", default = True), ) android_library_deps = android_library_deps + [lint_sources_target] lint( diff --git a/rules/android/lint/lint_aspect.bzl b/rules/android/lint/lint_aspect.bzl index c5164867..a882cc52 100644 --- a/rules/android/lint/lint_aspect.bzl +++ b/rules/android/lint/lint_aspect.bzl @@ -96,6 +96,8 @@ def _collect_sources(target, ctx, library): lint_config_xml = dep[AndroidLintSourcesInfo].lint_config[0], classpath = classpath, lint_checks = dep[AndroidLintSourcesInfo].lint_checks, + fail_on_warning = dep[AndroidLintSourcesInfo].fail_on_warning, + fail_on_information = dep[AndroidLintSourcesInfo].fail_on_information, ) for dep in (ctx.rule.attr.deps + getattr(ctx.rule.attr, "exports", [])) if AndroidLintSourcesInfo in dep @@ -355,6 +357,8 @@ def _lint_report_action( baseline, updated_baseline, lint_config_xml_file, + fail_on_warning, + fail_on_information, lint_result_xml_file, partial_results_dir, jdk_home, @@ -395,6 +399,8 @@ def _lint_report_action( ) args.add("--updated-baseline", updated_baseline) + args.add("--fail-on-warning", fail_on_warning) + args.add("--fail-on-information", fail_on_information) args.add("--output-xml", lint_result_xml_file.path) args.add("--result-code", result_code) @@ -564,6 +570,8 @@ def _lint_aspect_impl(target, ctx): baseline = sources.baseline, updated_baseline = lint_updated_baseline_file, lint_config_xml_file = sources.lint_config_xml, + fail_on_warning = sources.fail_on_warning, + fail_on_information = sources.fail_on_information, lint_result_xml_file = lint_result_xml_file, partial_results_dir = lint_results_dir, models_dir = lint_models_dir, diff --git a/rules/android/lint/lint_sources.bzl b/rules/android/lint/lint_sources.bzl index 4d155313..e30a3d0e 100644 --- a/rules/android/lint/lint_sources.bzl +++ b/rules/android/lint/lint_sources.bzl @@ -30,6 +30,8 @@ def _lint_sources_impl(ctx): baseline = _target_outputs([ctx.attr.baseline]) if ctx.attr.baseline != None else None, lint_config = _target_outputs([ctx.attr.lint_config]) if ctx.attr.lint_config != None else _target_outputs([ctx.attr._default_lint_config]), lint_checks = ctx.attr.lint_checks, + fail_on_warning = ctx.attr.fail_on_warning, + fail_on_information = ctx.attr.fail_on_information, ), ] @@ -71,6 +73,8 @@ lint_sources = rule( default = [], providers = [JavaInfo], ), + "fail_on_warning": attr.bool(default = True, doc = "exit code 1 if it find Lint Issues with severity of Warning", mandatory = False), + "fail_on_information": attr.bool(default = True, doc = "exit code 1 if it find Lint Issues with severity of Warning", mandatory = False), # TODO(arun) add assets }, provides = [ diff --git a/rules/android/lint/providers.bzl b/rules/android/lint/providers.bzl index 0bd09871..32c242c2 100644 --- a/rules/android/lint/providers.bzl +++ b/rules/android/lint/providers.bzl @@ -32,6 +32,8 @@ AndroidLintSourcesInfo = provider( baseline = "Lint baseline XML", lint_config = "Lint config XML", lint_checks = "Custom Lint Targets", + fail_on_warning = "fail on Lint issues with warning severity", + fail_on_information = "fail on Lint issues with information severity", ), ) diff --git a/rules/kotlin/kotlin.bzl b/rules/kotlin/kotlin.bzl index 944412c6..5599562f 100644 --- a/rules/kotlin/kotlin.bzl +++ b/rules/kotlin/kotlin.bzl @@ -24,6 +24,8 @@ def kt_jvm_library_interal( srcs = attrs.get("srcs"), baseline = lint_baseline, lint_config = lint_options.get("config", None), + fail_on_warning = lint_options.get("fail_on_warning", default = True), + fail_on_information = lint_options.get("fail_on_information", default = True), ) lint( diff --git a/tools/lint/src/main/java/com/grab/lint/LintReportCommand.kt b/tools/lint/src/main/java/com/grab/lint/LintReportCommand.kt index 8afd6ef8..ae786ad5 100644 --- a/tools/lint/src/main/java/com/grab/lint/LintReportCommand.kt +++ b/tools/lint/src/main/java/com/grab/lint/LintReportCommand.kt @@ -1,6 +1,7 @@ package com.grab.lint import com.github.ajalt.clikt.parameters.options.convert +import com.github.ajalt.clikt.parameters.options.default import com.github.ajalt.clikt.parameters.options.option import com.github.ajalt.clikt.parameters.options.required import java.io.File @@ -28,6 +29,18 @@ class LintReportCommand : LintBaseCommand() { help = "File containing result status of running Lint" ).convert { File(it) }.required() + private val failOnWarnings by option( + "-fow", + "--fail-on-warning", + help = "exit code 1 if it find Lint issues with severity of Warning" + ).convert { it.toBoolean() }.default(true) + + private val failOnInformation by option( + "-foi", + "--fail-on-information", + help = "exit code 1 if it find Lint issues with severity of Information" + ).convert { it.toBoolean() }.default(true) + override fun run( workingDir: Path, projectXml: File, @@ -37,7 +50,9 @@ class LintReportCommand : LintBaseCommand() { newBaseline.copyTo(updatedBaseline) LintResults( resultCodeFile = resultCode, - lintResultsFile = outputXml + lintResultsFile = outputXml, + failOnWarnings = failOnWarnings, + failOnInformation = failOnInformation, ).process() } diff --git a/tools/lint/src/main/java/com/grab/lint/LintResults.kt b/tools/lint/src/main/java/com/grab/lint/LintResults.kt index c8a1a494..01c256bb 100644 --- a/tools/lint/src/main/java/com/grab/lint/LintResults.kt +++ b/tools/lint/src/main/java/com/grab/lint/LintResults.kt @@ -8,6 +8,8 @@ import javax.xml.parsers.DocumentBuilderFactory class LintResults( val resultCodeFile: File, val lintResultsFile: File, + val failOnWarnings: Boolean = true, + val failOnInformation: Boolean = true, ) { private fun NodeList.elements() = (0 until length).map { item(it) as Element } @@ -17,7 +19,16 @@ class LintResults( val lintResults = documentBuilder.parse(lintResultsFile) val issues = lintResults.getElementsByTagName("issue") - val hasErrors = issues.elements().any { it.getAttribute("id") != "LintBaseline" } + val hasErrors = issues.elements().any { + it.getAttribute("id") != "LintBaseline" && + it.getAttribute("id") != "LintError" && + ( + it.getAttribute("severity") == "Fatal" || + it.getAttribute("severity") == "Error" || + (failOnWarnings && it.getAttribute("severity") == "Warning") || + (failOnInformation && it.getAttribute("severity") == "Information") + ) + } resultCodeFile.writeText(if (hasErrors) "1" else "0") } catch (e: Exception) { e.printStackTrace() diff --git a/tools/lint/src/test/java/com/grab/lint/LintResultsTest.kt b/tools/lint/src/test/java/com/grab/lint/LintResultsTest.kt index 9b055ca4..4d080311 100644 --- a/tools/lint/src/test/java/com/grab/lint/LintResultsTest.kt +++ b/tools/lint/src/test/java/com/grab/lint/LintResultsTest.kt @@ -55,7 +55,119 @@ class LintResultsTest { } @Test - fun `assert when any other error apart from LintBaseLine are there, result code is 1`() { + fun `assert when Fail for Information severity is true, result code is 1`() { + lintResults = LintResults(resultCode, resultXml, failOnInformation = true) + resultXml.writeText( + """ + + + + + + + + """.trimIndent() + ) + lintResults.process() + assertResultCode("1", "Result is 1 when additional errors are there") + } + + @Test + fun `assert when Fail for Information severity is false, result code is 0`() { + lintResults = LintResults(resultCode, resultXml, failOnInformation = false) + resultXml.writeText( + """ + + + + + + + + """.trimIndent() + ) + lintResults.process() + assertResultCode("0", "Result is 0 when Fail for Information severity is false") + } + + @Test + fun `assert when Fail for Warnings severity is true, result code is 1`() { + lintResults = LintResults(resultCode, resultXml, failOnWarnings = true) + resultXml.writeText( + """ + + + + + + + + """.trimIndent() + ) + lintResults.process() + assertResultCode("1", "Result is 1 when Fail for Warnings severity is true") + } + + @Test + fun `assert when Fail for Warning severity is false, result code is 0`() { + lintResults = LintResults(resultCode, resultXml, failOnWarnings = false) + resultXml.writeText( + """ + + + + + + + + """.trimIndent() + ) + lintResults.process() + assertResultCode("0", "Result is 0 when Fail for Warning severity is false") + } + + @Test + fun `assert when any other error apart from LintBaseLine and LintError are there, result code is 1`() { resultXml.writeText( """ @@ -72,6 +184,18 @@ class LintResultsTest { + + + + + + + + + + + + + + + """.trimIndent() + ) + lintResults.process() + assertResultCode("0", "Result is 0 when additional errors are there") + } + @Test fun `assert when result file is malformed or does not exist, result is 1`() { lintResults.process()