From e578f4ca93f6292f62e4b4c7fa11d17fe0b27c71 Mon Sep 17 00:00:00 2001 From: ByteHamster Date: Sun, 7 Apr 2024 23:28:14 +0200 Subject: [PATCH] CI tweaks (#7069) - Run Checkstyle with gradle to make it easier for users - No longer needs different configuration for new code - Exclude current violations - Fix some violations that somehow couldn't be specified in the exclusion file - Print SpotBugs/Lint/Checkstly violations in GitHub format - Then the CI run gets annotated on the web UI --- .github/pull_request_template.md | 1 + .github/workflows/checks.yml | 57 ++--- .github/workflows/errorPrinter.py | 48 ++++ CONTRIBUTING.md | 3 +- app/build.gradle | 10 +- .../feedgenerator/GeneratorUtil.java | 4 +- .../ui/screen/preferences/ProxyDialog.java | 6 +- .../ui/view/LockableBottomSheetBehavior.java | 3 +- .../layout/alternate_urls_dropdown_item.xml | 4 +- .../main/res/layout/alternate_urls_item.xml | 4 +- app/src/main/res/layout/bug_report.xml | 44 ++-- .../res/layout/checkbox_do_not_show_again.xml | 27 +- app/src/main/res/layout/edit_text_dialog.xml | 21 +- .../res/layout/ellipsize_start_listitem.xml | 6 +- app/src/main/res/layout/empty_view_layout.xml | 73 +++--- .../main/res/layout/feed_pref_skip_dialog.xml | 10 +- app/src/main/res/layout/secondary_action.xml | 48 ++-- ...ple_list_item_multiple_choice_on_start.xml | 7 +- .../main/res/layout/simplechapter_item.xml | 100 ++++---- config/checkstyle/checkstyle-new-code.xml | 235 ------------------ config/checkstyle/checkstyle.xml | 63 ++++- config/checkstyle/suppressions.xml | 19 +- net/common/build.gradle | 4 + .../antennapod/net/common/NetworkUtils.java | 4 +- .../danoeh/antennapod/net/common/UriUtil.java | 4 +- .../autodownload/APCleanupAlgorithm.java | 4 +- .../sync/gpoddernet/model/GpodnetPodcast.java | 15 +- .../playback/base/RewindAfterPauseUtils.java | 4 +- .../playback/service/PlaybackController.java | 3 +- storage/database/build.gradle | 2 +- .../storage/preferences/UserPreferences.java | 4 +- ui/common/build.gradle | 4 + .../antennapod/ui/common/IntentUtils.java | 4 +- ui/preferences/build.gradle | 10 - ui/preferences/src/main/assets/.gitignore | 4 - ui/preferences/src/main/assets/LICENSE.txt | 1 + .../src/main/res/layout/about_teaser.xml | 14 +- .../res/layout/choose_data_folder_dialog.xml | 15 +- .../choose_data_folder_dialog_entry.xml | 8 +- .../res/layout/gpodnetauth_device_row.xml | 19 +- .../main/res/layout/gpodnetauth_finish.xml | 4 +- .../main/res/layout/simple_icon_list_item.xml | 54 ++-- .../main/res/layout/statistics_fragment.xml | 35 +-- .../main/res/layout/statistics_listitem.xml | 121 ++++----- 44 files changed, 503 insertions(+), 627 deletions(-) create mode 100644 .github/workflows/errorPrinter.py delete mode 100644 config/checkstyle/checkstyle-new-code.xml delete mode 100644 ui/preferences/src/main/assets/.gitignore create mode 120000 ui/preferences/src/main/assets/LICENSE.txt diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index ef44d874d..bcf581997 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -9,6 +9,7 @@ --> - [ ] I have read the contribution guidelines: https://github.com/AntennaPod/AntennaPod/blob/develop/CONTRIBUTING.md#submit-a-pull-request - [ ] I have performed a self-review of my code +- [ ] I have run the automated code checks using `./gradlew checkstyle spotbugsPlayDebug spotbugsDebug :app:lintPlayDebug` - [ ] My code follows the style guidelines of the AntennaPod project: https://github.com/AntennaPod/AntennaPod/wiki/Code-style - [ ] I have mentioned the corresponding issue and the relevant keyword (e.g., "Closes: #xy") in the description (see https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue) - [ ] If it is a core feature, I have added automated tests diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 9dbfdcf1a..6431f3125 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -7,36 +7,8 @@ on: branches: [ master, develop ] jobs: - code-style: - name: "Code Style" - runs-on: ubuntu-latest - timeout-minutes: 45 - steps: - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - name: Checkstyle - run: | - curl -s -L https://github.com/checkstyle/checkstyle/releases/download/checkstyle-10.3.1/checkstyle-10.3.1-all.jar > checkstyle.jar - find . -name "*\.java" | xargs java -Dconfig_loc=config/checkstyle -jar checkstyle.jar -c config/checkstyle/checkstyle.xml - - name: Find PR Base Commit - id: vars - run: | - git fetch origin develop - echo "branchBaseCommit=$(git merge-base origin/develop HEAD)" >> $GITHUB_OUTPUT - - name: Diff-Checkstyle - run: | - curl -s -L https://github.com/yangziwen/diff-check/releases/download/0.0.7/diff-checkstyle.jar > diff-checkstyle.jar - java -Dconfig_loc=config/checkstyle -jar diff-checkstyle.jar -c config/checkstyle/checkstyle-new-code.xml --git-dir . --base-rev ${{ steps.vars.outputs.branchBaseCommit }} - - name: XML of changed files - run: | - curl -s -L https://github.com/ByteHamster/android-xml-formatter/releases/download/1.1.0/android-xml-formatter.jar > android-xml-formatter.jar - git diff --name-only ${{ steps.vars.outputs.branchBaseCommit }} --diff-filter=AM | { grep "res/layout/" || true; } | xargs java -jar android-xml-formatter.jar - test $(git diff | wc -l) -eq 0 || (echo -e "\n\n===== Found XML code style violations! See output below how to fix them. =====\n\n" && git --no-pager diff --color=always && false) - wrapper-validation: name: "Gradle Wrapper Validation" - needs: code-style runs-on: ubuntu-latest timeout-minutes: 45 steps: @@ -45,11 +17,12 @@ jobs: static-analysis: name: "Static Code Analysis" - needs: code-style runs-on: ubuntu-latest timeout-minutes: 45 steps: - uses: actions/checkout@v4 + with: + fetch-depth: 0 - name: Set up JDK 17 uses: actions/setup-java@v4 with: @@ -62,14 +35,24 @@ jobs: ~/.gradle/caches ~/.gradle/wrapper key: gradle-${{ hashFiles('**/*.gradle*') }}-${{ hashFiles('**/gradle/wrapper/gradle-wrapper.properties') }} - - name: Lint :app module recursively - run: ./gradlew :app:lintPlayRelease - - name: SpotBugs - run: ./gradlew spotbugsPlayDebug spotbugsDebug + - name: Configure parallel build + run: echo "org.gradle.parallel=true" >> local.properties + - name: XML code style + run: | + curl -s -L https://github.com/ByteHamster/android-xml-formatter/releases/download/1.1.0/android-xml-formatter.jar > android-xml-formatter.jar + find . -wholename "*/res/layout/*.xml" | xargs java -jar android-xml-formatter.jar + test $(git diff | wc -l) -eq 0 || (echo -e "\n\n===== Found XML code style violations! See output below how to fix them. =====\n\n" && git --no-pager diff --color=always && false) + - name: Checkstyle, Lint, SpotBugs + run: ./gradlew checkstyle :app:lintPlayDebug spotbugsPlayDebug spotbugsDebug + - name: Generate readable error messages for GitHub + if: failure() + run: | + git diff --name-only | xargs -I '{}' echo "::error file={},line=1,endLine=1,title=XML Format::Run android-xml-formatter.jar on this file or view CI output to see how it should be formatted." + python .github/workflows/errorPrinter.py unit-test: name: "Unit Test: ${{ matrix.variant }}" - needs: code-style + needs: static-analysis runs-on: ubuntu-latest timeout-minutes: 45 strategy: @@ -101,6 +84,8 @@ jobs: ~/.gradle/caches ~/.gradle/wrapper key: gradle-${{ hashFiles('**/*.gradle*') }}-${{ hashFiles('**/gradle/wrapper/gradle-wrapper.properties') }} + - name: Configure parallel build + run: echo "org.gradle.parallel=true" >> local.properties - name: Create temporary release keystore run: keytool -noprompt -genkey -v -keystore "app/keystore" -alias alias -storepass password -keypass password -keyalg RSA -validity 10 -dname "CN=antennapod.org, OU=dummy, O=dummy, L=dummy, S=dummy, C=US" - name: Build @@ -116,7 +101,7 @@ jobs: emulator-test: name: "Emulator Test" - needs: code-style + needs: static-analysis runs-on: macOS-latest timeout-minutes: 45 env: @@ -135,6 +120,8 @@ jobs: ~/.gradle/caches ~/.gradle/wrapper key: gradle-${{ hashFiles('**/*.gradle*') }}-${{ hashFiles('**/gradle/wrapper/gradle-wrapper.properties') }} + - name: Configure parallel build + run: echo "org.gradle.parallel=true" >> local.properties - name: Build with Gradle run: ./gradlew assemblePlayDebugAndroidTest - name: Android Emulator test diff --git a/.github/workflows/errorPrinter.py b/.github/workflows/errorPrinter.py new file mode 100644 index 000000000..c83b9b3ad --- /dev/null +++ b/.github/workflows/errorPrinter.py @@ -0,0 +1,48 @@ +#!/bin/python +from xml.dom import minidom +import os.path +import glob +from pathlib import Path + +if os.path.isfile('app/build/reports/lint-results-playDebug.xml'): + dom = minidom.parse('app/build/reports/lint-results-playDebug.xml') + issues = dom.getElementsByTagName('issue') + for issue in issues: + locations = issue.getElementsByTagName('location') + for location in locations: + print(location.attributes['file'].value + ":" + location.attributes['line'].value + " " + issue.attributes['summary'].value) + print("::error file=" + location.attributes['file'].value + + ",line=" + location.attributes['line'].value + + ",endLine=" + location.attributes['line'].value + + ",title=Lint::" + issue.attributes['summary'].value + ". " + issue.attributes['explanation'].value.replace('\n', ' ')) + print() + +if os.path.isfile('build/reports/checkstyle/checkstyle.xml'): + dom = minidom.parse('build/reports/checkstyle/checkstyle.xml') + files = dom.getElementsByTagName('file') + for f in files: + errors = f.getElementsByTagName('error') + for error in errors: + print(f.attributes['name'].value + ":" + error.attributes['line'].value + " " + error.attributes['message'].value) + print("::error file=" + f.attributes['name'].value + + ",line=" + error.attributes['line'].value + + ",endLine=" + error.attributes['line'].value + + ",title=Checkstyle::" + error.attributes['message'].value) + print() + + +for filename in glob.iglob('**/build/reports/spotbugs/*.xml', recursive=True): + filenamePath = Path(filename) + dom = minidom.parse(filename) + instance = dom.getElementsByTagName('BugInstance') + for inst in instance: + lines = inst.getElementsByTagName('SourceLine') + longMessage = inst.getElementsByTagName('LongMessage')[0].firstChild.nodeValue + for line in lines: + if "primary" in line.attributes: + print(line.attributes['sourcepath'].value + ": " + longMessage) + print("::error file=" + str(filenamePath.parent.parent.parent.parent.absolute()) + "/src/main/java/" + line.attributes['sourcepath'].value + + ",line=" + line.attributes['start'].value + + ",endLine=" + line.attributes['end'].value + + ",title=SpotBugs::" + longMessage.replace('\n', ' ')) + print() diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 201e98545..3c6ef5259 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -41,7 +41,8 @@ Submit a pull request - Get coding :) - If possible, add unit tests for your pull request and make sure that they pass. - Please do not upgrade dependencies or build tools unless you have a good reason for it. Doing so can easily introduce bugs that are hard to track down. - - Please follow our code style. You can use Checkstyle within Android Studio using our [configuration file](https://github.com/AntennaPod/AntennaPod/blob/develop/config/checkstyle/checkstyle-new-code.xml). + - Please follow our code style. You can use Checkstyle within Android Studio using our [configuration file](https://github.com/AntennaPod/AntennaPod/blob/develop/config/checkstyle/checkstyle.xml). + - To check the code style locally, run `./gradlew checkstyle spotbugsPlayDebug spotbugsDebug :app:lintPlayDebug` - Please only change the English string resources. Translations are handled on [Transifex](https://www.transifex.com/antennapod/antennapod/). - Open the PR - Mention the corresponding issue in the pull request text, so that it can be closed once your pull request has been merged. If you use [special keywords](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue), GitHub will close the issue(s) automatically. diff --git a/app/build.gradle b/app/build.gradle index 5b4fdf6b4..2abb66ccf 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -47,13 +47,11 @@ android { } lint { - disable 'ObsoleteLintCustomCheck', 'CheckResult', 'UnusedAttribute', 'BatteryLife', 'InflateParams', - 'RestrictedApi', 'TrustAllX509TrustManager', 'ExportedReceiver', 'AllowBackup', 'VectorDrawableCompat', - 'StaticFieldLeak', 'UseCompoundDrawables', 'NestedWeights', 'Overdraw', 'UselessParent', 'TextFields', - 'AlwaysShowAction', 'Autofill', 'ClickableViewAccessibility', 'ContentDescription', + disable 'CheckResult', 'MissingMediaBrowserServiceIntentFilter', 'UnusedAttribute', 'InflateParams', + 'RestrictedApi', 'ExportedReceiver', 'NotifyDataSetChanged', 'UseCompoundDrawables', 'NestedWeights', + 'Overdraw', 'UselessParent', 'TextFields', 'AlwaysShowAction', 'Autofill', 'ClickableViewAccessibility', 'KeyboardInaccessibleWidget', 'LabelFor', 'SetTextI18n', 'HardcodedText', 'RelativeOverlap', - 'RtlCompat', 'RtlHardcoded', 'MissingMediaBrowserServiceIntentFilter', 'VectorPath', - 'InvalidPeriodicWorkRequestInterval', 'NotifyDataSetChanged', 'RtlEnabled' + 'RtlHardcoded', 'RtlEnabled', 'ContentDescription' } androidResources { diff --git a/app/src/androidTest/java/de/test/antennapod/util/syndication/feedgenerator/GeneratorUtil.java b/app/src/androidTest/java/de/test/antennapod/util/syndication/feedgenerator/GeneratorUtil.java index 9aedbb493..d37ef3b07 100644 --- a/app/src/androidTest/java/de/test/antennapod/util/syndication/feedgenerator/GeneratorUtil.java +++ b/app/src/androidTest/java/de/test/antennapod/util/syndication/feedgenerator/GeneratorUtil.java @@ -7,9 +7,7 @@ import java.io.IOException; /** * Utility methods for FeedGenerator */ -class GeneratorUtil { - private GeneratorUtil(){} - +abstract class GeneratorUtil { public static void addPaymentLink(XmlSerializer xml, String paymentLink, boolean withNamespace) throws IOException { String ns = (withNamespace) ? "http://www.w3.org/2005/Atom" : null; xml.startTag(ns, "link"); diff --git a/app/src/main/java/de/danoeh/antennapod/ui/screen/preferences/ProxyDialog.java b/app/src/main/java/de/danoeh/antennapod/ui/screen/preferences/ProxyDialog.java index ca4b4b5e5..6086f5cb1 100644 --- a/app/src/main/java/de/danoeh/antennapod/ui/screen/preferences/ProxyDialog.java +++ b/app/src/main/java/de/danoeh/antennapod/ui/screen/preferences/ProxyDialog.java @@ -173,10 +173,12 @@ public class ProxyDialog { private final TextWatcher requireTestOnChange = new TextWatcher() { @Override - public void beforeTextChanged(CharSequence s, int start, int count, int after) {} + public void beforeTextChanged(CharSequence s, int start, int count, int after) { + } @Override - public void onTextChanged(CharSequence s, int start, int before, int count) {} + public void onTextChanged(CharSequence s, int start, int before, int count) { + } @Override public void afterTextChanged(Editable s) { diff --git a/app/src/main/java/de/danoeh/antennapod/ui/view/LockableBottomSheetBehavior.java b/app/src/main/java/de/danoeh/antennapod/ui/view/LockableBottomSheetBehavior.java index aa506aaea..13fcd2715 100644 --- a/app/src/main/java/de/danoeh/antennapod/ui/view/LockableBottomSheetBehavior.java +++ b/app/src/main/java/de/danoeh/antennapod/ui/view/LockableBottomSheetBehavior.java @@ -13,7 +13,8 @@ import com.google.android.material.bottomsheet.ViewPagerBottomSheetBehavior; public class LockableBottomSheetBehavior extends ViewPagerBottomSheetBehavior { private boolean isLocked = false; - public LockableBottomSheetBehavior() {} + public LockableBottomSheetBehavior() { + } public LockableBottomSheetBehavior(Context context, AttributeSet attrs) { super(context, attrs); diff --git a/app/src/main/res/layout/alternate_urls_dropdown_item.xml b/app/src/main/res/layout/alternate_urls_dropdown_item.xml index 82de8a02f..3c4dcc7fc 100644 --- a/app/src/main/res/layout/alternate_urls_dropdown_item.xml +++ b/app/src/main/res/layout/alternate_urls_dropdown_item.xml @@ -2,7 +2,7 @@ + android:paddingVertical="8dp" + style="?android:attr/spinnerDropDownItemStyle" /> diff --git a/app/src/main/res/layout/alternate_urls_item.xml b/app/src/main/res/layout/alternate_urls_item.xml index 9fdb50e33..54e05b49d 100644 --- a/app/src/main/res/layout/alternate_urls_item.xml +++ b/app/src/main/res/layout/alternate_urls_item.xml @@ -2,8 +2,8 @@ + android:textAlignment="inherit" + style="?android:attr/spinnerItemStyle" /> diff --git a/app/src/main/res/layout/bug_report.xml b/app/src/main/res/layout/bug_report.xml index e97e85265..447ce911f 100644 --- a/app/src/main/res/layout/bug_report.xml +++ b/app/src/main/res/layout/bug_report.xml @@ -1,28 +1,30 @@ - -