Add checkstyle

.github/workflows/ci.yml

@@ -40,3 +40,10 @@ jobs:
             echo running with mock downloader
             ./gradlew check --stacktrace -Ddownloader=MOCK
           fi
+
+      - name: Upload test reports when failure occurs
+        uses: actions/upload-artifact@v3
+        if: failure()
+        with:
+          name: NewPipeExtractor-test-reports
+          path: extractor/build/reports/tests/test/**

build.gradle

@@ -30,6 +30,7 @@ allprojects {
         nanojsonVersion = "1d9e1aea9049fc9f85e68b43ba39fe7be1c1f751"
         spotbugsVersion = "4.6.0"
         junitVersion = "5.8.2"
+        checkstyleVersion = "9.3" // do not use latest version (10.0) as it requires compile JDK 11
     }
 }
 

checkstyle/checkstyle.xml

@@ -0,0 +1,189 @@
+<?xml version="1.0"?>
+<!DOCTYPE module PUBLIC
+          "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
+          "https://checkstyle.org/dtds/configuration_1_3.dtd">
+<module name="Checker">
+    <!--
+        If you set the basedir property below, then all reported file
+        names will be relative to the specified directory. See
+        https://checkstyle.org/5.x/config.html#Checker
+
+        <property name="basedir" value="${basedir}"/>
+    -->
+    <property name="severity" value="error"/>
+
+    <property name="fileExtensions" value="java, properties, xml"/>
+
+    <!-- Excludes all 'module-info.java' files              -->
+    <!-- See https://checkstyle.org/config_filefilters.html -->
+    <module name="BeforeExecutionExclusionFileFilter">
+        <property name="fileNamePattern" value="module\-info\.java$"/>
+    </module>
+
+    <!-- https://checkstyle.org/config_filters.html#SuppressionFilter -->
+    <module name="SuppressionFilter">
+        <property name="file" value="${config_loc}/suppressions.xml" />
+        <property name="optional" value="true"/>
+    </module>
+
+    <!-- Checks that a package-info.java file exists for each package.     -->
+    <!-- See https://checkstyle.org/config_javadoc.html#JavadocPackage -->
+    <!--<module name="JavadocPackage"/>-->
+
+    <!-- Checks whether files end with a new line.                        -->
+    <!-- See https://checkstyle.org/config_misc.html#NewlineAtEndOfFile -->
+    <module name="NewlineAtEndOfFile"/>
+
+    <!-- Checks that property files contain the same keys.         -->
+    <!-- See https://checkstyle.org/config_misc.html#Translation -->
+    <module name="Translation"/>
+
+    <!-- Checks for Size Violations.                    -->
+    <!-- See https://checkstyle.org/config_sizes.html -->
+    <module name="FileLength"/>
+    <module name="LineLength">
+      <property name="max" value="100"/>
+      <property name="fileExtensions" value="java"/>
+    </module>
+
+    <!-- Checks for whitespace                               -->
+    <!-- See https://checkstyle.org/config_whitespace.html -->
+    <module name="FileTabCharacter"/>
+
+    <!-- Miscellaneous other checks.                   -->
+    <!-- See https://checkstyle.org/config_misc.html -->
+    <module name="RegexpSingleline">
+       <property name="format" value="\s+$"/>
+       <property name="minimum" value="0"/>
+       <property name="maximum" value="0"/>
+       <property name="message" value="Line has trailing spaces."/>
+    </module>
+
+    <!-- Checks for Headers                                -->
+    <!-- See https://checkstyle.org/config_header.html   -->
+    <!-- <module name="Header"> -->
+    <!--   <property name="headerFile" value="${checkstyle.header.file}"/> -->
+    <!--   <property name="fileExtensions" value="java"/> -->
+    <!-- </module> -->
+
+    <module name="SuppressWarningsFilter" />
+
+    <module name="TreeWalker">
+        <!-- Checks for Javadoc comments.                     -->
+        <!-- See https://checkstyle.org/config_javadoc.html -->
+        <module name="InvalidJavadocPosition"/>
+        <module name="JavadocMethod">
+            <property name="allowMissingParamTags" value="true"/>
+            <property name="allowMissingReturnTag" value="true"/>
+        </module>
+        <module name="JavadocType"/>
+        <!--<module name="JavadocVariable"/>-->
+        <module name="JavadocStyle">
+            <property name="checkFirstSentence" value="false"/>
+        </module>
+        <!--<module name="MissingJavadocMethod"/>-->
+
+        <!-- Checks for Naming Conventions.                  -->
+        <!-- See https://checkstyle.org/config_naming.html -->
+        <module name="ConstantName"/>
+        <module name="LocalFinalVariableName"/>
+        <module name="LocalVariableName"/>
+        <module name="MemberName">
+            <property name="format" value="^(TAG|DEBUG|[a-z][a-zA-Z0-9]*)$"/>
+        </module>
+        <module name="MethodName"/>
+        <module name="PackageName"/>
+        <module name="ParameterName"/>
+        <module name="StaticVariableName"/>
+        <module name="TypeName"/>
+
+        <!-- Checks for imports                              -->
+        <!-- See https://checkstyle.org/config_import.html -->
+        <module name="AvoidStarImport"/>
+        <module name="IllegalImport"/> <!-- defaults to sun.* packages -->
+        <module name="RedundantImport"/>
+        <module name="UnusedImports"/>
+
+        <!-- Checks for Size Violations.                    -->
+        <!-- See https://checkstyle.org/config_sizes.html -->
+        <module name="MethodLength">
+            <property name="severity" value="warning"/>
+        </module>
+        <module name="ParameterNumber">
+            <property name="severity" value="warning"/>
+        </module>
+
+        <!-- Checks for whitespace                               -->
+        <!-- See https://checkstyle.org/config_whitespace.html -->
+        <module name="EmptyForIteratorPad"/>
+        <module name="GenericWhitespace"/>
+        <module name="MethodParamPad"/>
+        <module name="NoWhitespaceAfter"/>
+        <module name="NoWhitespaceBefore"/>
+        <module name="OperatorWrap"/>
+        <module name="ParenPad"/>
+        <module name="TypecastParenPad"/>
+        <module name="WhitespaceAfter"/>
+        <module name="WhitespaceAround"/>
+
+        <!-- Modifier Checks                                    -->
+        <!-- See https://checkstyle.org/config_modifiers.html -->
+        <module name="ModifierOrder"/>
+        <module name="RedundantModifier"/>
+
+        <!-- Checks for blocks. You know, those {}'s         -->
+        <!-- See https://checkstyle.org/config_blocks.html -->
+        <module name="AvoidNestedBlocks"/>
+        <module name="EmptyBlock"/>
+        <module name="LeftCurly"/>
+        <module name="NeedBraces"/>
+        <module name="RightCurly"/>
+
+        <!-- Checks for common coding problems               -->
+        <!-- See https://checkstyle.org/config_coding.html -->
+        <module name="EmptyStatement"/>
+        <module name="EqualsHashCode">
+            <property name="severity" value="warning"/>
+        </module>
+        <module name="HiddenField">
+            <property name="ignoreConstructorParameter" value="true"/>
+            <property name="ignoreSetter" value="true"/>
+        </module>
+        <module name="IllegalInstantiation"/>
+        <module name="InnerAssignment"/>
+        <!--<module name="MagicNumber"/>-->
+        <!--<module name="MissingSwitchDefault">
+            <property name="severity" value="warning"/>
+        </module>-->
+        <module name="MultipleVariableDeclarations"/>
+        <module name="SimplifyBooleanExpression"/>
+        <module name="SimplifyBooleanReturn"/>
+        <module name="FinalLocalVariable">
+            <property name="tokens" value="VARIABLE_DEF,PARAMETER_DEF"/>
+            <property name="validateEnhancedForLoopVariable" value="true"/>
+        </module>
+
+        <!-- Checks for class design                         -->
+        <!-- See https://checkstyle.org/config_design.html -->
+        <!--<module name="DesignForExtension"/>-->
+        <module name="FinalClass"/>
+        <module name="HideUtilityClassConstructor"/>
+        <module name="InterfaceIsType"/>
+        <!--<module name="VisibilityModifier">
+            <property name="ignoreAnnotationCanonicalNames" value="State,ColumnInfo"/>
+            <property name="severity" value="warning"/>
+        </module>-->
+
+        <!-- Miscellaneous other checks.                   -->
+        <!-- See https://checkstyle.org/config_misc.html -->
+        <module name="ArrayTypeStyle"/>
+        <module name="FinalParameters"/>
+        <!--<module name="TodoComment">
+            <property name="format" value="(TODO:|FIXME:)"/>
+            <property name="severity" value="warning"/>
+        </module>-->
+        <module name="UpperEll"/>
+
+        <module name="SuppressWarningsHolder" />
+    </module>
+</module>

checkstyle/suppressions.xml

@@ -0,0 +1,15 @@
+<?xml version="1.0"?>
+<!DOCTYPE suppressions PUBLIC
+  "-//Checkstyle//DTD SuppressionFilter Configuration 1.2//EN"
+  "https://checkstyle.org/dtds/suppressions_1_2.dtd">
+<suppressions>
+  <!-- Use @SuppressWarnings("...") if it is possible, only use this file if it is not -->
+
+  <suppress checks="LineLength"
+    files="BandcampExtractorHelper.java"
+    lines="54"/>
+
+  <suppress checks="LineLength"
+    files="ItagItem.java"
+    lines="19"/>
+</suppressions>

extractor/build.gradle

@@ -1,9 +1,25 @@
+plugins {
+    id 'checkstyle'
+}
+
 test {
-//  Pass on downloader type to tests for different CI jobs. See DownloaderFactory.java and ci.yml
+    // Pass on downloader type to tests for different CI jobs. See DownloaderFactory.java and ci.yml
     if (System.properties.containsKey('downloader')) {
         systemProperty('downloader', System.getProperty('downloader'))
     }
     useJUnitPlatform()
+    dependsOn checkstyleMain // run checkstyle when testing
+}
+
+checkstyle {
+    getConfigDirectory().set(rootProject.file("checkstyle"))
+    ignoreFailures false
+    showViolations true
+    toolVersion checkstyleVersion
+}
+
+checkstyleTest {
+    enabled false // do not checkstyle test files
 }
 
 dependencies {
@@ -15,6 +31,8 @@ dependencies {
     implementation "com.github.spotbugs:spotbugs-annotations:$spotbugsVersion"
     implementation 'org.nibor.autolink:autolink:0.10.0'
 
+    checkstyle "com.puppycrawl.tools:checkstyle:$checkstyleVersion"
+
     testImplementation platform("org.junit:junit-bom:$junitVersion")
     testImplementation 'org.junit.jupiter:junit-jupiter-api'
     testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine'

extractor/src/main/java/org/schabi/newpipe/extractor/Extractor.java

@@ -10,13 +10,15 @@
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
+
 import java.io.IOException;
 import java.util.Objects;
 
 public abstract class Extractor {
     /**
      * {@link StreamingService} currently related to this extractor.<br>
-     * Useful for getting other things from a service (like the url handlers for cleaning/accepting/get id from urls).
+     * Useful for getting other things from a service (like the url handlers for
+     * cleaning/accepting/get id from urls).
      */
     private final StreamingService service;
     private final LinkHandler linkHandler;
@@ -27,16 +29,18 @@ public abstract class Extractor {
     private ContentCountry forcedContentCountry = null;
 
     private boolean pageFetched = false;
+    // called like this to prevent checkstyle errors about "hiding a field"
     private final Downloader downloader;
 
-    public Extractor(final StreamingService service, final LinkHandler linkHandler) {
+    protected Extractor(final StreamingService service, final LinkHandler linkHandler) {
         this.service = Objects.requireNonNull(service, "service is null");
         this.linkHandler = Objects.requireNonNull(linkHandler, "LinkHandler is null");
         this.downloader = Objects.requireNonNull(NewPipe.getDownloader(), "downloader is null");
     }
 
     /**
-     * @return The {@link LinkHandler} of the current extractor object (e.g. a ChannelExtractor should return a channel url handler).
+     * @return The {@link LinkHandler} of the current extractor object (e.g. a ChannelExtractor
+     *         should return a channel url handler).
      */
     @Nonnull
     public LinkHandler getLinkHandler() {
@@ -50,13 +54,17 @@ public LinkHandler getLinkHandler() {
      * @throws ExtractionException if the pages content is not understood
      */
     public void fetchPage() throws IOException, ExtractionException {
-        if (pageFetched) return;
+        if (pageFetched) {
+            return;
+        }
         onFetchPage(downloader);
         pageFetched = true;
     }
 
     protected void assertPageFetched() {
-        if (!pageFetched) throw new IllegalStateException("Page is not fetched. Make sure you call fetchPage()");
+        if (!pageFetched) {
+            throw new IllegalStateException("Page is not fetched. Make sure you call fetchPage()");
+        }
     }
 
     protected boolean isPageFetched() {
@@ -66,11 +74,13 @@ protected boolean isPageFetched() {
     /**
      * Fetch the current page.
      *
-     * @param downloader the download to use
+     * @param downloader the downloader to use
      * @throws IOException         if the page can not be loaded
      * @throws ExtractionException if the pages content is not understood
      */
-    public abstract void onFetchPage(@Nonnull Downloader downloader) throws IOException, ExtractionException;
+    @SuppressWarnings("HiddenField")
+    public abstract void onFetchPage(@Nonnull Downloader downloader)
+            throws IOException, ExtractionException;
 
     @Nonnull
     public String getId() throws ParsingException {
@@ -118,11 +128,11 @@ public Downloader getDownloader() {
     // Localization
     //////////////////////////////////////////////////////////////////////////*/
 
-    public void forceLocalization(Localization localization) {
+    public void forceLocalization(final Localization localization) {
         this.forcedLocalization = localization;
     }
 
-    public void forceContentCountry(ContentCountry contentCountry) {
+    public void forceContentCountry(final ContentCountry contentCountry) {
         this.forcedContentCountry = contentCountry;
     }
 
@@ -133,7 +143,8 @@ public Localization getExtractorLocalization() {
 
     @Nonnull
     public ContentCountry getExtractorContentCountry() {
-        return forcedContentCountry == null ? getService().getContentCountry() : forcedContentCountry;
+        return forcedContentCountry == null ? getService().getContentCountry()
+                : forcedContentCountry;
     }
 
     @Nonnull