Merge pull request #166 from grab/adding-control-over-lint-exit-code-…

…for-warning-and-information-issues

adding control over lint exit code for warning and information issues

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

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(

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,

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 = [

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",
     ),
 )
 

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(

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()
     }
 

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()

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(
+            """
+            <?xml version="1.0" encoding="UTF-8"?>
+            <issues format="6" by="lint 8.0.2">
+                <issue
+                    id="LintBaselineFixed"
+                    severity="Information"
+                    message="3 errors/warnings were listed in the baseline file (../../../../../../../../../../../../../../var/folders/jg/73xbn5_922b8h5l97xqqx9b40000gn/T/tmp792374699895101884/baseline.xml) but not found in the project; perhaps they have been fixed? Unmatched issue types: ExtraTranslation, PrivateResource, UnusedResources"
+                    category="Lint"
+                    priority="10"
+                    summary="Baselined Issues Fixed"
+                    explanation="If a lint baseline describes a problem which is no longer reported, then the problem has either been fixed, or perhaps the issue type has been disabled. In any case, the entry can be removed from the baseline (such that if the issue is reintroduced at some point, lint will complain rather than just silently starting to match the old baseline entry again.)">
+                    <location
+                        file="../../../../../../../../../../../../../../var/folders/jg/73xbn5_922b8h5l97xqqx9b40000gn/T/tmp792374699895101884/baseline.xml"/>
+                </issue>
+
+            </issues>
+        """.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(
+            """
+            <?xml version="1.0" encoding="UTF-8"?>
+            <issues format="6" by="lint 8.0.2">
+                <issue
+                    id="LintBaselineFixed"
+                    severity="Information"
+                    message="3 errors/warnings were listed in the baseline file (../../../../../../../../../../../../../../var/folders/jg/73xbn5_922b8h5l97xqqx9b40000gn/T/tmp792374699895101884/baseline.xml) but not found in the project; perhaps they have been fixed? Unmatched issue types: ExtraTranslation, PrivateResource, UnusedResources"
+                    category="Lint"
+                    priority="10"
+                    summary="Baselined Issues Fixed"
+                    explanation="If a lint baseline describes a problem which is no longer reported, then the problem has either been fixed, or perhaps the issue type has been disabled. In any case, the entry can be removed from the baseline (such that if the issue is reintroduced at some point, lint will complain rather than just silently starting to match the old baseline entry again.)">
+                    <location
+                        file="../../../../../../../../../../../../../../var/folders/jg/73xbn5_922b8h5l97xqqx9b40000gn/T/tmp792374699895101884/baseline.xml"/>
+                </issue>
+
+            </issues>
+        """.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(
+            """
+            <?xml version="1.0" encoding="UTF-8"?>
+            <issues format="6" by="lint 8.0.2">
+                <issue
+                    id="PrivateResource"
+                    severity="Warning"
+                    message="The resource `@color/material_blue_grey_950` is marked as private in material-1.2.1.aar"
+                    category="Correctness"
+                    priority="3"
+                    summary="Using private resources"
+                    explanation="Private resources should not be referenced; the may not be present everywhere, and even where they are they may disappear without notice.&#xA;&#xA;To fix this, copy the resource into your own project instead."
+                    errorLine1="    val material_res = com.google.android.material.R.color.material_blue_grey_950"
+                    errorLine2="                                                           ~~~~~~~~~~~~~~~~~~~~~~">
+                    <location
+                        file="../../../../../../../tests/android/binary/src/main/java/com/grab/test/TestActivity.kt"
+                        line="9"
+                        column="60"/>
+                </issue>
+
+            </issues>
+        """.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(
+            """
+            <?xml version="1.0" encoding="UTF-8"?>
+            <issues format="6" by="lint 8.0.2">
+                <issue
+                    id="PrivateResource"
+                    severity="Warning"
+                    message="The resource `@color/material_blue_grey_950` is marked as private in material-1.2.1.aar"
+                    category="Correctness"
+                    priority="3"
+                    summary="Using private resources"
+                    explanation="Private resources should not be referenced; the may not be present everywhere, and even where they are they may disappear without notice.&#xA;&#xA;To fix this, copy the resource into your own project instead."
+                    errorLine1="    val material_res = com.google.android.material.R.color.material_blue_grey_950"
+                    errorLine2="                                                           ~~~~~~~~~~~~~~~~~~~~~~">
+                    <location
+                        file="../../../../../../../tests/android/binary/src/main/java/com/grab/test/TestActivity.kt"
+                        line="9"
+                        column="60"/>
+                </issue>
+
+            </issues>
+        """.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(
             """
             <?xml version="1.0" encoding="UTF-8"?>
@@ -72,6 +184,18 @@ class LintResultsTest {
                     <location
                         file="baseline.xml"/>
                 </issue>
+                
+                <issue
+                    id="LintError"
+                    severity="Information"
+                    message="7 warnings were filtered out because they are listed in the baseline file, baseline.xml&#xA;"
+                    category="Lint"
+                    priority="10"
+                    summary="Baseline Issues"
+                    explanation="Lint can be configured with a &quot;baseline&quot;; a set of current issues found in a codebase, which future runs of lint will silently ignore. Only new issues not found in the baseline are reported.&#xA;&#xA;Note that while opening files in the IDE, baseline issues are not filtered out; the purpose of baselines is to allow you to get started using lint and break the build on all newly introduced errors, without having to go back and fix the entire codebase up front. However, when you open up existing files you still want to be aware of and fix issues as you come across them.&#xA;&#xA;This issue type is used to emit two types of informational messages in reports: first, whether any issues were filtered out so you don&apos;t have a false sense of security if you forgot that you&apos;ve checked in a baseline file, and second, whether any issues in the baseline file appear to have been fixed such that you can stop filtering them out and get warned if the issues are re-introduced.">
+                    <location
+                        file="baseline.xml"/>
+                </issue>
 
                 <issue
                     id="LongLogTag"
@@ -94,6 +218,43 @@ class LintResultsTest {
         assertResultCode("1", "Result is 1 when additional errors are there")
     }
 
+    @Test
+    fun `assert when there is no erro other than LintBaseLine and LintError, result code is 0`() {
+        resultXml.writeText(
+            """
+            <?xml version="1.0" encoding="UTF-8"?>
+            <issues format="6" by="lint 8.0.2">
+
+                <issue
+                    id="LintBaseline"
+                    severity="Information"
+                    message="7 warnings were filtered out because they are listed in the baseline file, baseline.xml&#xA;"
+                    category="Lint"
+                    priority="10"
+                    summary="Baseline Issues"
+                    explanation="Lint can be configured with a &quot;baseline&quot;; a set of current issues found in a codebase, which future runs of lint will silently ignore. Only new issues not found in the baseline are reported.&#xA;&#xA;Note that while opening files in the IDE, baseline issues are not filtered out; the purpose of baselines is to allow you to get started using lint and break the build on all newly introduced errors, without having to go back and fix the entire codebase up front. However, when you open up existing files you still want to be aware of and fix issues as you come across them.&#xA;&#xA;This issue type is used to emit two types of informational messages in reports: first, whether any issues were filtered out so you don&apos;t have a false sense of security if you forgot that you&apos;ve checked in a baseline file, and second, whether any issues in the baseline file appear to have been fixed such that you can stop filtering them out and get warned if the issues are re-introduced.">
+                    <location
+                        file="baseline.xml"/>
+                </issue>
+                
+                <issue
+                    id="LintError"
+                    severity="Information"
+                    message="7 warnings were filtered out because they are listed in the baseline file, baseline.xml&#xA;"
+                    category="Lint"
+                    priority="10"
+                    summary="Baseline Issues"
+                    explanation="Lint can be configured with a &quot;baseline&quot;; a set of current issues found in a codebase, which future runs of lint will silently ignore. Only new issues not found in the baseline are reported.&#xA;&#xA;Note that while opening files in the IDE, baseline issues are not filtered out; the purpose of baselines is to allow you to get started using lint and break the build on all newly introduced errors, without having to go back and fix the entire codebase up front. However, when you open up existing files you still want to be aware of and fix issues as you come across them.&#xA;&#xA;This issue type is used to emit two types of informational messages in reports: first, whether any issues were filtered out so you don&apos;t have a false sense of security if you forgot that you&apos;ve checked in a baseline file, and second, whether any issues in the baseline file appear to have been fixed such that you can stop filtering them out and get warned if the issues are re-introduced.">
+                    <location
+                        file="baseline.xml"/>
+                </issue>
+            </issues>
+        """.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()