diff --git a/.sonarcloud.properties b/.sonarcloud.properties new file mode 100644 index 0000000000..1869956068 --- /dev/null +++ b/.sonarcloud.properties @@ -0,0 +1 @@ +sonar.exclusions=**/integTest/resources/*.* diff --git a/subprojects/build.gradle.kts b/subprojects/build.gradle.kts index 385777e739..3f46f6cd23 100644 --- a/subprojects/build.gradle.kts +++ b/subprojects/build.gradle.kts @@ -270,6 +270,20 @@ subprojects { tasks.withType { systemProperty("rootDir", "${project.rootDir}") + + val testProperties = listOf( + "avito.slack.test.channel", + "avito.slack.test.token", + "avito.slack.test.workspace" + ) + testProperties.forEach { key -> + val property = if (project.hasProperty(key)) { + project.property(key)!!.toString() + } else { + "" + } + systemProperty(key, property) + } } } diff --git a/subprojects/gradle/cd/src/main/kotlin/com/avito/ci/steps/LintCheck.kt b/subprojects/gradle/cd/src/main/kotlin/com/avito/ci/steps/LintCheck.kt index f5591f9317..199cb91d5b 100644 --- a/subprojects/gradle/cd/src/main/kotlin/com/avito/ci/steps/LintCheck.kt +++ b/subprojects/gradle/cd/src/main/kotlin/com/avito/ci/steps/LintCheck.kt @@ -14,19 +14,28 @@ import org.gradle.kotlin.dsl.invoke class LintCheck(context: String, name: String) : SuppressibleBuildStep(context, name), ImpactAnalysisAwareBuildStep by ImpactAnalysisAwareBuildStep.Impl() { + var slackChannelForAlerts = "" + override fun registerTask(project: Project, rootTask: TaskProvider) { require(project.isAndroidApp()) { "Lint check can be applied only to android application modules" } + if (project.buildEnvironment !is BuildEnvironment.CI) return + if (useImpactAnalysis && !project.internalModule.isModified()) return project.pluginManager.withPlugin("com.avito.android.lint-report") { val lintReports = project.tasks.withType() + lintReports.configureEach { + if (slackChannelForAlerts.isNotBlank()) { + it.slackReportChannel.set(slackChannelForAlerts) + } it.abortOnError.set(!suppressFailures) } + lintReports.forEach { task -> task.onlyIf { !useImpactAnalysis || project.internalModule.lintConfiguration.isModified } rootTask { diff --git a/subprojects/gradle/lint-report/build.gradle.kts b/subprojects/gradle/lint-report/build.gradle.kts index 4df622fe31..6989473dd7 100644 --- a/subprojects/gradle/lint-report/build.gradle.kts +++ b/subprojects/gradle/lint-report/build.gradle.kts @@ -3,6 +3,7 @@ plugins { id("java-gradle-plugin") `maven-publish` id("com.jfrog.bintray") + id("nebula.integtest") } dependencies { @@ -13,14 +14,15 @@ dependencies { implementation(Dependencies.kotlinHtml) implementation(Dependencies.okhttp) - implementation(project(":gradle:android")) - implementation(project(":gradle:ci-logger")) implementation(project(":common:okhttp")) - implementation(project(":gradle:impact-shared")) implementation(project(":common:sentry")) - implementation(project(":gradle:git")) + implementation(project(":gradle:android")) implementation(project(":gradle:bitbucket")) + implementation(project(":gradle:ci-logger")) + implementation(project(":gradle:git")) + implementation(project(":gradle:impact-shared")) implementation(project(":gradle:kotlin-dsl-support")) + implementation(project(":gradle:slack")) testImplementation(project(":gradle:logging-test-fixtures")) } diff --git a/subprojects/gradle/lint-report/src/integTest/kotlin/com/avito/android/lint/LintSlackAlertIntegrationTest.kt b/subprojects/gradle/lint-report/src/integTest/kotlin/com/avito/android/lint/LintSlackAlertIntegrationTest.kt new file mode 100644 index 0000000000..9ab5c319fb --- /dev/null +++ b/subprojects/gradle/lint-report/src/integTest/kotlin/com/avito/android/lint/LintSlackAlertIntegrationTest.kt @@ -0,0 +1,27 @@ +package com.avito.android.lint + +import com.avito.slack.SlackClient +import com.avito.slack.model.SlackChannel +import com.avito.utils.logging.CILogger +import org.junit.jupiter.api.Test +import java.io.File + +internal class LintSlackAlertIntegrationTest { + + private val testChannel = SlackChannel(requireNotNull(System.getProperty("avito.slack.test.channel"))) + private val testToken = requireNotNull(System.getProperty("avito.slack.test.token")) + private val workspace = requireNotNull(System.getProperty("avito.slack.test.workspace")) + private val slackClient: SlackClient = SlackClient.Impl(testToken, workspace) + private val logger: CILogger = CILogger.allToStdout + + @Test + fun integrationTest() { + val parser = LintResultsParser(File("src/integTest/resources"), logger) + + val reportModels = parser.parse() + + val lintSlackReporter: LintSlackReporter = LintSlackReporter.Impl(slackClient, logger) + + lintSlackReporter.report(reportModels, testChannel) + } +} diff --git a/subprojects/gradle/lint-report/src/integTest/resources/lint-results.html b/subprojects/gradle/lint-report/src/integTest/resources/lint-results.html new file mode 100644 index 0000000000..c9b8e95fe3 --- /dev/null +++ b/subprojects/gradle/lint-report/src/integTest/resources/lint-results.html @@ -0,0 +1,1534 @@ + + + + + +Lint Report + + + + + + + + +
+
+
+ Lint Report: 28 errors +
+ +
+
+
+ Issue Types + +
+
+
+ +
+
+
+

Overview

+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
Correctness +
1error +DefaultLocale: Implied default locale in case conversion
Performance +
1error +ObsoleteLayoutParam: Obsolete layout params
3error +MergeRootFrame: FrameLayout can be replaced with <merge> tag
1error +InefficientWeight: Inefficient layout weight
1error +UnusedResources: Unused resources
Usability:Icons +
1error +IconLocation: Image defined in density-independent drawable folder
Usability +
2error +TextFields: Missing inputType
3error +Autofill: Use Autofill
Accessibility +
2error +LabelFor: Missing accessibility label
Internationalization +
12error +HardcodedText: Hardcoded text
Internationalization:Bidirectional Text +
1error +RtlHardcoded: Using left/right instead of start/end attributes
Disabled Checks (40) +
+
+
+
+
+
+ + +
+
+
+

Implied default locale in case conversion

+
+
+
+
+../../src/main/kotlin/com/avito/android/ui/RecyclerAsLayoutActivity.kt:65: Implicitly using the default locale is a common source of bugs: Use toUpperCase(Locale) instead. For strings meant to be internal use Locale.ROOT, otherwise Locale.getDefault().
+ 62 
+ 63       override fun getItemCount() = hints.size
+ 64 
+ 65       override fun getItemViewType(position: Int): Int = ViewType.valueOf(hints[position].toUpperCase()).ordinal
+ 66   }
+ 67 
+ 68   enum class ViewType { INPUT, EDIT, LABEL }
+
+ +
+
+
+
+
+ + DefaultLocale + + + Correctness + + + Error + + + Priority 6/10 + +
+
+
+
+
+
+ + +
+
+
+

Obsolete layout params

+
+
+
+
+../../src/main/res/layout/activity_buttons_over_recycler.xml:44: Invalid layout param in a RelativeLayout: layout_weight
+ 41             android:layout_height="wrap_content"
+ 42             android:layout_alignParentBottom="true"
+ 43             android:layout_centerHorizontal="true"
+ 44             android:layout_weight="1"                                                               
+ 45             android:enabled="true"
+ 46             android:text="Do not press" />
+
+ +
+
+
+
+
+ + ObsoleteLayoutParam + + + Performance + + + Error + + + Priority 6/10 + +
+
+
+
+
+
+
+
+
+

FrameLayout can be replaced with <merge> tag

+
+
+
+
+../../src/main/res/layout/activity_empty.xml:2: This <FrameLayout> can be replaced with a <merge> tag
+ 1 <?xml version="1.0" encoding="utf-8"?>
+ 2 <FrameLayout xmlns:android="http://schemas.android.com/apk/res/android"                             
+ 3     android:id="@+id/root"
+ 4     android:layout_width="match_parent"
+ 5     android:layout_height="match_parent">
+
+ +../../src/main/res/layout/activity_moving_button.xml:2: This <FrameLayout> can be replaced with a <merge> tag
+  1 <?xml version="1.0" encoding="utf-8"?>
+  2 <FrameLayout xmlns:android="http://schemas.android.com/apk/res/android"                             
+  3     android:layout_width="match_parent"
+  4     android:layout_height="match_parent">
+
+ +../../src/main/res/layout/activity_overlap.xml:3: This <FrameLayout> can be replaced with a <merge> tag
+  1 <?xml version="1.0" encoding="utf-8"?>
+  2 
+  3 <FrameLayout xmlns:android="http://schemas.android.com/apk/res/android"                             
+  4     xmlns:tools="http://schemas.android.com/tools"
+  5     android:id="@+id/root"
+  6     android:layout_width="match_parent"
+ +
+
+
+
+
+ + MergeRootFrame + + + Performance + + + Error + + + Priority 4/10 + +
+
+
+
+
+
+
+
+
+

Inefficient layout weight

+
+
+
+
+../../src/main/res/layout/activity_buttons_over_recycler.xml:24: Use a layout_width of 0dp instead of wrap_content for better performance
+ 21         <Button
+ 22             android:id="@+id/top_button"
+ 23             style="@style/Widget.AppCompat.Button.Colored"
+ 24             android:layout_width="wrap_content"                                                     
+ 25             android:layout_height="wrap_content"
+ 26             android:layout_marginLeft="4dp"
+ 27             android:layout_weight="1"
+ +
+
+
+
+
+ + InefficientWeight + + + Performance + + + Error + + + Priority 3/10 + +
+
+
+
+
+
+
+
+
+

Unused resources

+
+
+
+
+../../src/main/res/layout/activity_app_bar.xml:3: The resource R.layout.activity_app_bar appears to be unused
+  1 <?xml version="1.0" encoding="utf-8"?>
+  2 
+  3 <androidx.coordinatorlayout.widget.CoordinatorLayout xmlns:android="http://schemas.android.com/apk/res/android"
+  4     xmlns:app="http://schemas.android.com/apk/res-auto"
+  5     xmlns:tools="http://schemas.android.com/tools"
+  6     android:id="@+id/wallet_page_activity_root"
+ +
+
+
+
+
+ + UnusedResources + + + Performance + + + Error + + + Priority 3/10 + +
+
+
+
+
+
+ + +
+
+
+

Image defined in density-independent drawable folder

+
+
+
+
+../../src/main/res/drawable/red_bitmap.png: Found bitmap drawable res/drawable/red_bitmap.png in densityless folder
+
+
+
+
+
+ + IconLocation + + + Icons + + + Usability + + + Error + + + Priority 5/10 + +
+
+
+
+
+
+ + +
+
+
+

Missing inputType

+
+
+
+
+../../src/main/res/layout/activity_edittext.xml:9: This text field does not specify an inputType
+  6     android:layout_height="match_parent"
+  7     android:orientation="vertical">
+  8 
+  9     <EditText                                                                                       
+ 10         android:id="@+id/edit_text"
+ 11         android:layout_width="match_parent"
+ 12         android:layout_height="wrap_content"
+ +../../src/main/res/layout/activity_edittext.xml:23: This text field does not specify an inputType
+ 20         android:text="Test2"
+ 21         tools:ignore="HardcodedText" />
+ 22 
+ 23     <EditText                                                                                       
+ 24         android:id="@+id/phone_number_edit_text1"
+ 25         android:layout_width="match_parent"
+ 26         android:layout_height="wrap_content" />
+
+ +
+
+
+
+
+ + TextFields + + + Usability + + + Error + + + Priority 5/10 + +
+
+
+
+
+
+
+
+
+

Use Autofill

+
+
+
+
+../../src/main/res/layout/activity_edittext.xml:9: Missing autofillHints attribute
+  6     android:layout_height="match_parent"
+  7     android:orientation="vertical">
+  8 
+  9     <EditText                                                                                       
+ 10         android:id="@+id/edit_text"
+ 11         android:layout_width="match_parent"
+ 12         android:layout_height="wrap_content"
+ +../../src/main/res/layout/activity_edittext.xml:23: Missing autofillHints attribute
+ 20         android:text="Test2"
+ 21         tools:ignore="HardcodedText" />
+ 22 
+ 23     <EditText                                                                                       
+ 24         android:id="@+id/phone_number_edit_text1"
+ 25         android:layout_width="match_parent"
+ 26         android:layout_height="wrap_content" />
+
+ +../../src/main/res/layout/cell_with_edit_text.xml:9: Missing autofillHints attribute
+  6     android:layout_width="match_parent"
+  7     android:layout_height="wrap_content">
+  8 
+  9     <EditText                                                                                       
+ 10         android:id="@+id/edit_text"
+ 11         android:layout_width="match_parent"
+ 12         android:layout_height="wrap_content"
+ +
+
+
+
+
+ + Autofill + + + Usability + + + Error + + + Priority 3/10 + +
+
+
+
+
+
+ + +
+
+
+

Missing accessibility label

+
+
+
+
+../../src/main/res/layout/activity_edittext.xml:9: Missing accessibility label: provide either a view with an android:labelFor that references this view or provide an android:hint
+  6     android:layout_height="match_parent"
+  7     android:orientation="vertical">
+  8 
+  9     <EditText                                                                                       
+ 10         android:id="@+id/edit_text"
+ 11         android:layout_width="match_parent"
+ 12         android:layout_height="wrap_content"
+ +../../src/main/res/layout/activity_edittext.xml:23: Missing accessibility label: provide either a view with an android:labelFor that references this view or provide an android:hint
+ 20         android:text="Test2"
+ 21         tools:ignore="HardcodedText" />
+ 22 
+ 23     <EditText                                                                                       
+ 24         android:id="@+id/phone_number_edit_text1"
+ 25         android:layout_width="match_parent"
+ 26         android:layout_height="wrap_content" />
+
+ +
+
+
+
+
+ + LabelFor + + + Accessibility + + + Error + + + Priority 2/10 + +
+
+
+
+
+
+ + +
+
+
+

Hardcoded text

+
+
+
+
+../../src/main/res/layout/activity_buttons_over_recycler.xml:29: Hardcoded string "Do not press", should use @string resource
+ 26             android:layout_marginLeft="4dp"
+ 27             android:layout_weight="1"
+ 28             android:enabled="true"
+ 29             android:text="Do not press" />                                                          
+ 30 
+ 31     </LinearLayout>
+
+ +../../src/main/res/layout/activity_buttons_over_recycler.xml:46: Hardcoded string "Do not press", should use @string resource
+ 43             android:layout_centerHorizontal="true"
+ 44             android:layout_weight="1"
+ 45             android:enabled="true"
+ 46             android:text="Do not press" />                                                          
+ 47 
+ 48     </RelativeLayout>
+
+ +../../src/main/res/layout/activity_buttons_over_recycler_with_collapsing_toolbar.xml:26: Hardcoded string "Do not press", should use @string resource
+ 23             android:layout_alignParentBottom="true"
+ 24             android:layout_centerHorizontal="true"
+ 25             android:enabled="true"
+ 26             android:text="Do not press" />                                                          
+ 27 
+ 28     </RelativeLayout>
+
+ +../../src/main/res/layout/activity_distant_view_on_scroll.xml:21: Hardcoded string "Test", should use @string resource
+ 18             android:id="@+id/view"
+ 19             android:layout_width="match_parent"
+ 20             android:layout_height="wrap_content"
+ 21             android:text="Test" />                                                                  
+ 22 
+ 23     </LinearLayout>
+
+ +../../src/main/res/layout/activity_drawable.xml:48: Hardcoded string "text", should use @string resource
+ 45         android:layout_width="wrap_content"
+ 46         android:layout_height="wrap_content"
+ 47         android:gravity="center"
+ 48         android:text="text"                                                                         
+ 49         tools:drawableRight="@drawable/ic_check_black_24dp"
+ 50         tools:ignore="MissingPrefix" />
+
+ + + +
+
+
+
+
+ + HardcodedText + + + Internationalization + + + Error + + + Priority 5/10 + +
+
+
+
+
+
+ + +
+
+
+

Using left/right instead of start/end attributes

+
+
+
+
+../../src/main/res/layout/activity_buttons_over_recycler.xml:26: Consider replacing android:layout_marginLeft with android:layout_marginStart="4dp" to better support right-to-left layouts
+ 23             style="@style/Widget.AppCompat.Button.Colored"
+ 24             android:layout_width="wrap_content"
+ 25             android:layout_height="wrap_content"
+ 26             android:layout_marginLeft="4dp"                                                         
+ 27             android:layout_weight="1"
+ 28             android:enabled="true"
+ 29             android:text="Do not press" />
+
+ +
+
+
+
+
+ + RtlHardcoded + + + Bidirectional Text + + + Internationalization + + + Error + + + Priority 5/10 + +
+
+
+
+
+
+ +
+
+
+

Disabled Checks

+
+
+One or more issues were not run by lint, either +because the check is not enabled by default, or because +it was disabled with a command line flag or via one or +more lint.xml configuration files in the project directories. +
+
+
+
+
+ +
+
+
+

Suppressing Warnings and Errors

+
+
+Lint errors can be suppressed in a variety of ways:
+
+1. With a @SuppressLint annotation in the Java code
+2. With a tools:ignore attribute in the XML file
+3. With a //noinspection comment in the source code
+4. With ignore flags specified in the build.gradle file, as explained below
+5. With a lint.xml configuration file in the project
+6. With a lint.xml configuration file passed to lint via the --config flag
+7. With the --ignore flag passed to lint.
+
+To suppress a lint warning with an annotation, add a @SuppressLint("id") annotation on the class, method or variable declaration closest to the warning instance you want to disable. The id can be one or more issue id's, such as "UnusedResources" or {"UnusedResources","UnusedIds"}, or it can be "all" to suppress all lint warnings in the given scope.
+
+To suppress a lint warning with a comment, add a //noinspection id comment on the line before the statement with the error.
+
+To suppress a lint warning in an XML file, add a tools:ignore="id" attribute on the element containing the error, or one of its surrounding elements. You also need to define the namespace for the tools prefix on the root element in your document, next to the xmlns:android declaration:
+xmlns:tools="http://schemas.android.com/tools"
+
+To suppress a lint warning in a build.gradle file, add a section like this:
+ +
+android {
+    lintOptions {
+        disable 'TypographyFractions','TypographyQuotes'
+    }
+}
+
+
+Here we specify a comma separated list of issue id's after the disable command. You can also use warning or error instead of disable to change the severity of issues.
+
+To suppress lint warnings with a configuration XML file, create a file named lint.xml and place it at the root directory of the module in which it applies.
+
+The format of the lint.xml file is something like the following:
+ +
+<?xml version="1.0" encoding="UTF-8"?>
+<lint>
+    <!-- Ignore everything in the test source set -->
+    <issue id="all">
+        <ignore path="\*/test/\*" />
+    </issue>
+
+    <!-- Disable this given check in this project -->
+    <issue id="IconMissingDensityFolder" severity="ignore" />
+
+    <!-- Ignore the ObsoleteLayoutParam issue in the given files -->
+    <issue id="ObsoleteLayoutParam">
+        <ignore path="res/layout/activation.xml" />
+        <ignore path="res/layout-xlarge/activation.xml" />
+        <ignore regexp="(foo|bar)\.java" />
+    </issue>
+
+    <!-- Ignore the UselessLeaf issue in the given file -->
+    <issue id="UselessLeaf">
+        <ignore path="res/layout/main.xml" />
+    </issue>
+
+    <!-- Change the severity of hardcoded strings to "error" -->
+    <issue id="HardcodedText" severity="error" />
+</lint>
+
+
+To suppress lint checks from the command line, pass the --ignore flag with a comma separated list of ids to be suppressed, such as:
+$ lint --ignore UnusedResources,UselessLeaf /my/project/path
+
+For more information, see http://g.co/androidstudio/suppressing-lint-warnings
+ +
+
+
+
+
+ + diff --git a/subprojects/gradle/lint-report/src/integTest/resources/lint-results.xml b/subprojects/gradle/lint-report/src/integTest/resources/lint-results.xml new file mode 100644 index 0000000000..17429c7d34 --- /dev/null +++ b/subprojects/gradle/lint-report/src/integTest/resources/lint-results.xml @@ -0,0 +1,464 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintReportExtension.kt b/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintReportExtension.kt new file mode 100644 index 0000000000..0bbca727cf --- /dev/null +++ b/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintReportExtension.kt @@ -0,0 +1,13 @@ +package com.avito.android.lint + +import org.gradle.api.model.ObjectFactory +import org.gradle.kotlin.dsl.property +import javax.inject.Inject + +open class LintReportExtension @Inject constructor(objects: ObjectFactory) { + + val slackToken = objects.property() + + val slackWorkspace = objects.property() + +} diff --git a/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintReportMerger.kt b/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintReportMerger.kt index 89aab045b9..1572e409b6 100644 --- a/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintReportMerger.kt +++ b/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintReportMerger.kt @@ -22,14 +22,10 @@ import kotlinx.html.tr import kotlinx.html.unsafe import java.io.File -internal class LintReportMerger( - private val reports: List, - private val mergedReport: File -) { +class LintReportMerger { - fun write() { - val document = createHtmlDocument(sort(reports)) - mergedReport.writeText(document) + fun merge(reports: List, mergedReport: File): String { + return createHtmlDocument(sort(reports), mergedReport) } private fun sort(models: List): List { @@ -43,7 +39,7 @@ internal class LintReportMerger( return invalidReports + validReports.sortedWith(sortComparator) } - private fun createHtmlDocument(reports: List): String { + private fun createHtmlDocument(reports: List, mergedReport: File): String { return createHTML().html { head { meta { @@ -84,7 +80,7 @@ internal class LintReportMerger( } main("mdl-layout__content") { div("page-content") { - generatePageContent(this, reports) + generatePageContent(this, reports, mergedReport) } } } @@ -92,7 +88,7 @@ internal class LintReportMerger( } } - private fun generatePageContent(container: HtmlBlockTag, reports: List) { + private fun generatePageContent(container: HtmlBlockTag, reports: List, mergedReport: File) { container.table("mdl-data-table mdl-shadow--2dp") { thead { tr { diff --git a/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintReportModel.kt b/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintReportModel.kt index 4a3fa39375..a3a612fd9e 100644 --- a/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintReportModel.kt +++ b/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintReportModel.kt @@ -2,7 +2,7 @@ package com.avito.android.lint import java.io.File -internal sealed class LintReportModel(val projectRelativePath: String, val htmlFile: File) { +sealed class LintReportModel(val projectRelativePath: String, val htmlFile: File) { class Valid( projectRelativePath: String, @@ -17,7 +17,9 @@ internal sealed class LintReportModel(val projectRelativePath: String, val htmlF ) : LintReportModel(projectRelativePath, htmlFile) } -internal class LintIssue( +class LintIssue( + val id: String, + val summary: String, val message: String, val path: String, val line: Int, diff --git a/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintReportPlugin.kt b/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintReportPlugin.kt index f58a292ccd..810f712521 100644 --- a/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintReportPlugin.kt +++ b/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintReportPlugin.kt @@ -5,6 +5,7 @@ import com.avito.android.withAndroidApp import com.avito.bitbucket.Bitbucket import com.avito.bitbucket.atlassianCredentials import com.avito.bitbucket.bitbucketConfig +import com.avito.slack.SlackClient import com.avito.utils.gradle.BuildEnvironment import com.avito.utils.gradle.buildEnvironment import com.avito.utils.gradle.envArgs @@ -13,7 +14,9 @@ import org.gradle.api.Plugin import org.gradle.api.Project import org.gradle.api.Task import org.gradle.api.UnknownTaskException +import org.gradle.api.provider.Provider import org.gradle.api.tasks.TaskProvider +import org.gradle.kotlin.dsl.create import org.gradle.kotlin.dsl.invoke import org.gradle.kotlin.dsl.register @@ -23,8 +26,21 @@ open class LintReportPlugin : Plugin { override fun apply(app: Project) { check(app.isAndroidApp()) { "Plugin must be applied to an application but was applied to ${app.path}" } + val extension = app.extensions.create("lintReport") + + @Suppress("UnstableApiUsage") + val slackClientProvider: Provider = + extension.slackToken.zip(extension.slackWorkspace) { token, workspace -> + SlackClient.Impl( + token = token, + workspace = workspace + ) + } + val reportTask = app.tasks.register("lintReport") { + this.slackClient.set(slackClientProvider) + val atlassianCredentials = app.atlassianCredentials if (atlassianCredentials.isPresent) { bitbucket.set( diff --git a/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintReportTask.kt b/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintReportTask.kt index 334a76dfe8..fe34afb574 100644 --- a/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintReportTask.kt +++ b/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintReportTask.kt @@ -6,6 +6,8 @@ import com.avito.bitbucket.Severity import com.avito.git.gitState import com.avito.impact.configuration.internalModule import com.avito.kotlin.dsl.getMandatoryStringProperty +import com.avito.slack.SlackClient +import com.avito.slack.model.SlackChannel import com.avito.utils.logging.ciLogger import okhttp3.HttpUrl import okhttp3.HttpUrl.Companion.toHttpUrl @@ -27,9 +29,15 @@ abstract class LintReportTask : DefaultTask(), BuildVerdictTask { @get:Input abstract val buildId: Property + @get:Input + abstract val slackReportChannel: Property + @get:Internal abstract val bitbucket: Property + @get:Internal + abstract val slackClient: Property + @OutputDirectory val reportsDir: File = project.file(project.buildDir.path + "/reports/lint/modules") @@ -45,10 +53,20 @@ abstract class LintReportTask : DefaultTask(), BuildVerdictTask { copyReports() val models = parseReports() sendToBitbucket(models) + if (slackReportChannel.isPresent) { + createLintSlackAlert().report(models, SlackChannel(slackReportChannel.get())) + } val report = mergeReports(models) failIfNeeded(models, report) } + private fun createLintSlackAlert(): LintSlackReporter { + return LintSlackReporter.Impl( + slackClient = slackClient.get(), + logger = ciLogger + ) + } + private fun cleanOldReports() { if (reportsDir.exists()) { reportsDir.deleteRecursively() @@ -87,11 +105,12 @@ abstract class LintReportTask : DefaultTask(), BuildVerdictTask { private fun parseReports(): List = LintResultsParser(reportsDir, project.ciLogger).parse() private fun mergeReports(models: List): File { - val report = project.file(project.buildDir.path + "/reports/lint/lint-merge.html") - LintReportMerger(models, report).write() + val reportFile = project.file(project.buildDir.path + "/reports/lint/lint-merge.html") + val mergedReport = LintReportMerger().merge(models, reportFile) + reportFile.writeText(mergedReport) - project.ciLogger.debug("Wrote HTML report to file://${report.canonicalPath}") - return report + project.ciLogger.debug("Wrote HTML report to file://${reportFile.canonicalPath}") + return reportFile } private fun sendToBitbucket(reports: List) { diff --git a/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintResultsParser.kt b/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintResultsParser.kt index 1437c2d12a..1345aec42f 100644 --- a/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintResultsParser.kt +++ b/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintResultsParser.kt @@ -6,7 +6,7 @@ import java.io.InputStream import javax.xml.namespace.QName import javax.xml.stream.XMLInputFactory -internal class LintResultsParser( +class LintResultsParser( private val reportsDir: File, private val log: CILogger ) { @@ -34,6 +34,8 @@ internal class LintResultsParser( } private data class MutableLintIssue( + var id: String = "", + var summary: String = "", var message: String = "", var severity: LintIssue.Severity = LintIssue.Severity.UNKNOWN, var path: String = "", @@ -63,6 +65,8 @@ internal class LintResultsParser( if (startElement.name.localPart == "issue") { issue = MutableLintIssue() + issue.id = requireNotNull(startElement.getAttributeByName(QName("id"))?.value) + issue.summary = requireNotNull(startElement.getAttributeByName(QName("summary"))?.value) issue.message = startElement.getAttributeByName(QName("message"))?.value ?: "No message" issue.severity = when (startElement.getAttributeByName(QName("severity"))?.value) { "Error" -> LintIssue.Severity.ERROR @@ -84,7 +88,9 @@ internal class LintResultsParser( if (endElement.name.localPart == "issue") { issues.add( LintIssue( - message = issue!!.message, + id = issue!!.id, + summary = issue.summary, + message = issue.message, severity = issue.severity, line = issue.line, path = issue.path.replace("../", "") diff --git a/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintSlackReporter.kt b/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintSlackReporter.kt new file mode 100644 index 0000000000..7a846c78f2 --- /dev/null +++ b/subprojects/gradle/lint-report/src/main/kotlin/com/avito/android/lint/LintSlackReporter.kt @@ -0,0 +1,65 @@ +package com.avito.android.lint + +import com.avito.slack.SlackClient +import com.avito.slack.model.SlackChannel +import com.avito.utils.logging.CILogger + +interface LintSlackReporter { + + fun report( + models: List, + channel: SlackChannel + ) + + class Impl( + private val slackClient: SlackClient, + private val logger: CILogger + ) : LintSlackReporter { + + private val tag = "LintSlackAlert" + + override fun report( + models: List, + channel: SlackChannel + ) { + models.forEach { model -> + if (shouldSendAlert(model)) { + logger.debug("$tag: Sending lint alert...") + + slackClient.uploadHtml( + channel = channel, + message = buildSlackMessage(model), + file = model.htmlFile + ).fold( + { logger.debug("$tag: Report sent successfully") }, + { error -> logger.critical("$tag: Can't send report ${error.message}") } + ) + } else { + logger.debug("$tag Skip sending lint alert") + } + } + } + + private fun shouldSendAlert(model: LintReportModel): Boolean { + return (model is LintReportModel.Valid) && model.issues.any { it.severity == LintIssue.Severity.ERROR } + } + + private fun buildSlackMessage(model: LintReportModel): String { + return buildString { + appendln("*Critical lint problems detected!*") + appendln("for project: ${model.projectRelativePath}") + + if (model is LintReportModel.Valid) { + val errors = model.issues.filter { it.severity == LintIssue.Severity.ERROR } + val groupedErrors = errors.groupBy { it.summary } + groupedErrors.forEach { (summary, issue) -> + appendln(":red_circle: [${issue.size}x] $summary") + } + appendln(":warning: also ${model.issues.count { it.severity == LintIssue.Severity.WARNING }} warnings") + } else { + logger.critical("LintSlackAlerter: There is a problem with report: ${model.htmlFile.path}") + } + } + } + } +} diff --git a/subprojects/gradle/slack-test-fixtures/src/main/kotlin/com/avito/slack/FakeSlackClient.kt b/subprojects/gradle/slack-test-fixtures/src/main/kotlin/com/avito/slack/FakeSlackClient.kt index a5d978b37f..163f35ad3f 100644 --- a/subprojects/gradle/slack-test-fixtures/src/main/kotlin/com/avito/slack/FakeSlackClient.kt +++ b/subprojects/gradle/slack-test-fixtures/src/main/kotlin/com/avito/slack/FakeSlackClient.kt @@ -6,6 +6,7 @@ import com.avito.slack.model.SlackMessage import com.avito.slack.model.SlackSendMessageRequest import com.avito.slack.model.createStubInstance import org.funktionale.tries.Try +import java.io.File class FakeSlackClient : SlackClient { @@ -57,4 +58,8 @@ class FakeSlackClient : SlackClient { updatedMessageTimestamp = messageTimestamp return Try.Success(SlackMessage.createStubInstance()) } + + override fun uploadHtml(channel: SlackChannel, message: String, file: File): Try { + TODO("Not yet implemented") + } } diff --git a/subprojects/gradle/slack/build.gradle.kts b/subprojects/gradle/slack/build.gradle.kts index 8c719732aa..068e2841a9 100644 --- a/subprojects/gradle/slack/build.gradle.kts +++ b/subprojects/gradle/slack/build.gradle.kts @@ -18,21 +18,3 @@ dependencies { testImplementation(project(":gradle:slack-test-fixtures")) testImplementation(project(":common:time-test-fixtures")) } - -tasks.withType(Test::class.java).forEach { testTask -> - with(testTask) { - val testProperties = listOf( - "avito.slack.test.channel", - "avito.slack.test.token", - "avito.slack.test.workspace" - ) - testProperties.forEach { key -> - val property = if (project.hasProperty(key)) { - project.property(key)!!.toString() - } else { - "" - } - systemProperty(key, property) - } - } -} diff --git a/subprojects/gradle/slack/src/main/kotlin/com/avito/slack/SlackClient.kt b/subprojects/gradle/slack/src/main/kotlin/com/avito/slack/SlackClient.kt index 235ca0c77f..73fb2ab4de 100644 --- a/subprojects/gradle/slack/src/main/kotlin/com/avito/slack/SlackClient.kt +++ b/subprojects/gradle/slack/src/main/kotlin/com/avito/slack/SlackClient.kt @@ -14,8 +14,9 @@ import com.github.seratch.jslack.api.methods.request.chat.ChatPostMessageRequest import com.github.seratch.jslack.api.methods.request.chat.ChatUpdateRequest import com.github.seratch.jslack.api.model.Channel import org.funktionale.tries.Try +import java.io.File -interface SlackClient : SlackMessageSender { +interface SlackClient : SlackMessageSender, SlackFileUploader { fun updateMessage( channel: SlackChannel, @@ -62,6 +63,18 @@ interface SlackClient : SlackMessageSender { } } + override fun uploadHtml(channel: SlackChannel, message: String, file: File): Try { + return findChannelByName(channel.strippedName).flatMap { channelInfo -> + methodsClient.filesUpload { + it.file(file) + it.channels(listOf(channelInfo.id)) + it.filename(file.name) + it.initialComment(message) + it.token(token) + }.toTry() + }.map { Unit } + } + override fun updateMessage( channel: SlackChannel, text: String, @@ -149,7 +162,6 @@ private fun T.toTry(): Try { return if (this.isOk) { Try.Success(this) } else { - Try.Failure(RuntimeException("Slack request failed: ${this.error}")) + Try.Failure(RuntimeException("Slack request failed; error=${this.error} [needed=${this.needed}; provided=${this.provided}]")) } } - diff --git a/subprojects/gradle/slack/src/main/kotlin/com/avito/slack/SlackFileUploader.kt b/subprojects/gradle/slack/src/main/kotlin/com/avito/slack/SlackFileUploader.kt new file mode 100644 index 0000000000..ac2fad357a --- /dev/null +++ b/subprojects/gradle/slack/src/main/kotlin/com/avito/slack/SlackFileUploader.kt @@ -0,0 +1,14 @@ +package com.avito.slack + +import com.avito.slack.model.SlackChannel +import org.funktionale.tries.Try +import java.io.File + +interface SlackFileUploader { + + fun uploadHtml( + channel: SlackChannel, + message: String, + file: File + ): Try +}