Rollback aggressive linting (#4003)

- Revise linting rules
  - Make lint failures opt-in (for the project) instead of opt-out
  - Reduce noise
- Add explicit errors for things we would ask somebody to change in a
code review
  - Update baseline to only include the new errors
  - Remove baseline autoupdate task since:
    - We want this to happen very rarely
    - The autoupdater also adds warnings
- Remove reviewdog github action (that autoadds lint comments to PRs)

.github/workflows/ktlint.yml

@@ -1,35 +0,0 @@
-name: reviewdog-suggester
-on: pull_request
-jobs:
-  ktlint:
-    timeout-minutes: 5
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v3
-
-      - uses: actions/setup-java@v3
-        with:
-          distribution: 'corretto'
-          java-version: '17'
-          cache: 'gradle'
-
-      - run: mkdir -p ~/.gradle ; cp .github/ci-gradle.properties ~/.gradle/gradle.properties
-
-      - uses: gradle/wrapper-validation-action@v1
-
-      - uses: gradle/gradle-build-action@v2
-        with:
-          cache-read-only: ${{ github.ref != 'refs/heads/main' && github.ref != 'refs/heads/develop' }}
-
-      - run: chmod +x ./gradlew
-
-      - run: ./gradlew ktlintFormat
-
-      - uses: reviewdog/action-suggester@v1
-        with:
-          tool_name: ktlintFormat
-
-permissions:
-  contents: read
-  issues: write
-  pull-requests: write

CONTRIBUTING.md

@@ -22,8 +22,9 @@ We try to follow the [Guide to app architecture](https://developer.android.com/t
 
 ### Kotlin
 Tusky was originally written in Java, but is in the process of migrating to Kotlin. All new code must be written in Kotlin.
-We try to follow the [Kotlin Style Guide](https://developer.android.com/kotlin/style-guide) and make format the code according to the default [ktlint codestyle](https://github.com/pinterest/ktlint).
+We try to follow the [Kotlin Style Guide](https://developer.android.com/kotlin/style-guide) and format the code according to the default [ktlint codestyle](https://github.com/pinterest/ktlint).
-You can check the codestyle by running `./gradlew ktlintCheck`.
+You can check the codestyle by running `./gradlew ktlintCheck lint`. This will fail if you have any errors, and produces a detailed report which also lists warnings.
+We intentionally have very few hard linting errors, so that new contributors can focus on what they want to achieve instead of fighting the linter.
 
 ### Text
 All English text that will be visible to users must be put in `app/src/main/res/values/strings.xml` so it is translateable into other languages.

app/build.gradle

@@ -202,18 +202,3 @@ tasks.withType(org.jetbrains.kotlin.gradle.internal.KaptWithoutKotlincTask) {
         "--add-opens", "jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED",
         "--add-opens", "jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED"])
 }
-
-tasks.register("newLintBaseline") {
-    description 'Deletes and then recreates the lint baseline'
-
-    // This task should always run, irrespective of caching
-    notCompatibleWithConfigurationCache("Is always out of date")
-    outputs.upToDateWhen { false }
-
-    doLast {
-        delete android.lint.baseline.path
-    }
-
-    // Regenerate the lint baseline
-    it.finalizedBy tasks.named("lintBlueDebug")
-}

app/lint.xml

@@ -29,6 +29,7 @@
 
          Disable these for the time being. -->
     <issue id="UnusedIds" severity="ignore" />
+    <issue id="UnusedResources" severity="ignore" />
 
     <!-- Logs are stripped in release builds. -->
     <issue id="LogConditional" severity="ignore" />
@@ -36,19 +37,39 @@
     <!-- Newer dependencies are handled by Renovate, and don't need a warning -->
     <issue id="GradleDependency" severity="ignore" />
 
-    <!-- Typographical quotes are not something we care about at the moment -->
+    <!-- Typographical punctuation is not something we care about at the moment -->
     <issue id="TypographyQuotes" severity="ignore" />
+    <issue id="TypographyDashes" severity="ignore" />
+    <issue id="TypographyEllipsis" severity="ignore" />
 
-    <!-- Ensure we are warned about errors in the baseline -->
+    <!-- Translations come from external parties -->
-    <issue id="LintBaseline" severity="warning" />
+    <issue id="MissingQuantity" severity="ignore" />
+    <issue id="ImpliedQuantity" severity="ignore" />
+    <!-- Most alleged typos are in translations -->
+    <issue id="Typos" severity="ignore" />
 
-    <!-- Warn about typos. The typo database in lint is not exhaustive, and it's unclear
+    <!-- Basically all of our vectors are external -->
-         how to add to it when it's wrong. -->
+    <issue id="VectorPath" severity="ignore" />
-    <issue id="Typos" severity="warning" />
+    <issue id="Overdraw" severity="ignore" />
 
-    <!-- Set OldTargetApi back to warning -->
+    <!-- Irrelevant api version warnings -->
-    <issue id="OldTargetApi" severity="warning" />
+    <issue id="OldTargetApi" severity="ignore" />
+    <issue id="UnusedAttribute" severity="ignore" />
 
-    <!-- Mark all other lint issues as errors -->
+    <!-- We do not *want* all the text in the app to be selectable -->
-    <issue id="all" severity="error" />
+    <issue id="SelectableText" severity="ignore" />
+
+    <!-- This is heavily used by the viewbinding helper -->
+    <issue id="SyntheticAccessor" severity="ignore" />
+
+    <!-- Things we would actually question in a code review -->
+    <issue id="MissingPermission" severity="error" />
+    <issue id="InvalidPackage" severity="error" />
+    <issue id="UseCompatLoadingForDrawables" severity="error" />
+    <issue id="UseCompatTextViewDrawableXml" severity="error" />
+    <issue id="Recycle" severity="error" />
+    <issue id="KeyboardInaccessibleWidget" severity="error" />
+
+    <!-- Mark all other lint issues as warnings -->
+    <issue id="all" severity="warning" />
 </lint>