Skip to content

Commit

Permalink
Cleanup excessive linter checks (adoptium#2609)
Browse files Browse the repository at this point in the history
* Cleanup excessive linter checks

Commiters have identified that the super linter checks are excessively strict, so this pr attempts to summarise and eliminate checks that appear non-blocking to the Adoptium community

Signed-off-by: Morgan Davies <[email protected]>

* Disable rules for build template

* They're mostly to do with how the build template is lain out which doesn't matter as values get replaced when it is executed

Signed-off-by: Morgan Davies <[email protected]>

* Cleanup of markdown files

A lot of these are simple changes but the rules that were flagged are valid issues  imo (the only one not being so is MD029 which is handled automatically by github so there is no need to enforce it)

Signed-off-by: Morgan Davies <[email protected]>

* Disable shellcheck on build template

This file is modified by build.sh so it shouldn't really be linted

Signed-off-by: Morgan Davies <[email protected]>

* Linter regex exclude pointing to wrong location

* Add lang tag to Azure Readme

* Add initial checkstyle java linter config

* This allows us to exclude and configure linter rules

* Add jscpd config

* This is warning us about code clones in the test files which are not actually clones as they implement different parameters

Signed-off-by: Morgan Davies <[email protected]>

* Disable the more strict java rules

* Common standards are to include a comment on why they have been disabled

Signed-off-by: Morgan Davies <[email protected]>

* Commenting the rules out didn't work, try disabling them using the xml class

* Java config file should have underscore

* Expand jscpd to cover backward paths too

* Use a suppression config file

* Missing doctype declaration and correct format

* Add filepaths

* Move suppression filter out of treewalker

* Cleanup reamaining shell errors

* Ignore tab warnings
* Tabs and spaces generally do the same thing

Signed-off-by: Morgan Davies <[email protected]>

* Fix CudaEnabled & FeatureTest lint issues

* Fix JdkPlatform & StreamUtils lint jobs

* Missing periods and final keyword

* Disable perl linting for now

* Spawned an issue to address seperatly

* Revert enable of VALIDATE_ALL_CODEBASE

* Delete java-tool

* Hasn't been touched in 3 years
* Generating lots of errors
  • Loading branch information
Morgan Davies authored May 5, 2021
1 parent aa9ac3c commit 0d358c4
Show file tree
Hide file tree
Showing 21 changed files with 374 additions and 1,432 deletions.
13 changes: 6 additions & 7 deletions .azure-devops/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ Supported Versions/Platforms:
| jdk15 hotspot | ✔️ | ✔️ ||
| jdk-tip hotspot| ✔️ | ✔️ ||


## Requirements

1. Azure DevOps Account
Expand All @@ -22,11 +21,11 @@ If you don't have an Azure DevOps organization, you can start from

2. Required Pipeline Variables:

1. `JAVA_TO_BUILD`: jdk8u | jdk11u | jdk14u | jdk15u | jdk
- `JAVA_TO_BUILD`: jdk8u | jdk11u | jdk14u | jdk15u | jdk

3. Optional Pipeline Variables:

1. `EXTRA_MAKEJDK_ANY_PLATFORM_OPTIONS`: other options in makejdk-any-platform.sh
- `EXTRA_MAKEJDK_ANY_PLATFORM_OPTIONS`: other options in makejdk-any-platform.sh

## Quick Start

Expand All @@ -36,7 +35,7 @@ If you don't have an Azure DevOps organization, you can start from

2. Create a new Pipeline by using an **Existing Azure Pipeline YAML file**
and choose `.azure-devops/pipelines.yml` file.

Plase set the `JAVA_TO_BUILD` variable in the review step.

3. Start the pipeline and you can download the artifacts once the jobs complete.
Expand Down Expand Up @@ -75,7 +74,7 @@ This may add a maintenance cost over time as upstream changes may conflicts with

Second, you can create another YAML step template and save it to `build/shared/set_filename.yml` and add it to the `build.yml` file.

```
```yml
steps:
- template: ./steps/shared/before.yml
- template: ./steps/shared/set_filename.yml
Expand All @@ -86,8 +85,8 @@ steps:
By doing this you will not get any merge conflicts when you pull the changes from upstream.
<!---
<!---
Links.
--->
[azdo_main]: https://azure.microsoft.com/en-ca/services/devops/
[azdo_make_project_public]: https://docs.microsoft.com/en-us/azure/devops/organizations/public/make-project-public
[azdo_make_project_public]: https://docs.microsoft.com/en-us/azure/devops/organizations/public/make-project-public
11 changes: 11 additions & 0 deletions .github/linters/.jscpd.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"threshold": 0,
"reporters": [
"consoleFull"
],
"ignore": [
"**/__snapshots__/**",
"**/test/**"
],
"absolute": true
}
3 changes: 2 additions & 1 deletion .github/linters/.markdown-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@
MD013: false # Line length is usually not important and the 80 char limit is way too small anyway
MD026: false # Trailing punctuation in header
MD033: false # Inline HTML is important for multilines and checkboxes within markdown tables
MD034: false # no-bare-urls Bare URL used
MD034: false # no-bare-urls Bare URL used
MD029: false # Markdown automatically calculates the ordered list order
186 changes: 186 additions & 0 deletions .github/linters/sun_checks.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
"-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
"https://checkstyle.org/dtds/configuration_1_3.dtd">

<!--
Checkstyle configuration that checks the sun coding conventions from:
- the Java Language Specification at
https://docs.oracle.com/javase/specs/jls/se11/html/index.html
- the Sun Code Conventions at https://www.oracle.com/java/technologies/javase/codeconventions-contents.html
- the Javadoc guidelines at
https://www.oracle.com/technical-resources/articles/java/javadoc-tool.html
- the JDK Api documentation https://docs.oracle.com/en/java/javase/11/
- some best practices
Checkstyle is very configurable. Be sure to read the documentation at
https://checkstyle.org (or in your downloaded distribution).
Most Checks are configurable, be sure to consult the documentation.
To completely disable a check, just comment it out or delete it from the file.
To suppress certain violations please review suppression filters.
Finally, it is worth reading the documentation.
-->

<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/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="${org.checkstyle.sun.suppressionfilter.config}" default="checkstyle-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"/>

<!-- 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>

<!-- Enables @SuppressWarnings Support -->
<module name="SuppressWarningsFilter"/>

<!-- Suppress checks with config file -->
<module name="SuppressionFilter">
<property name="file" value=".github/linters/suppressed-java.xml"/>
</module>

<module name="TreeWalker">

<!-- Checks for Javadoc comments. -->
<!-- See https://checkstyle.org/config_javadoc.html -->
<module name="InvalidJavadocPosition"/>
<module name="JavadocMethod"/>
<module name="JavadocType"/>
<module name="JavadocStyle"/>
<module name="MissingJavadocMethod"/>
<!-- Enables @SuppressWarnings Support -->
<module name="SuppressWarningsHolder"/>

<!-- Checks for Naming Conventions. -->
<!-- See https://checkstyle.org/config_naming.html -->
<module name="ConstantName"/>
<module name="LocalFinalVariableName"/>
<module name="LocalVariableName"/>
<module name="MemberName"/>
<module name="MethodName"/>
<module name="PackageName"/>
<module name="ParameterName"/>
<module name="StaticVariableName"/>
<module name="TypeName"/>

<!-- Checks for imports -->
<!-- See https://checkstyle.org/config_imports.html -->
<module name="AvoidStarImport"/>
<module name="IllegalImport"/>
<!-- defaults to sun.* packages -->
<module name="RedundantImport"/>
<module name="UnusedImports">
<property name="processJavadoc" value="false"/>
</module>

<!-- Checks for Size Violations. -->
<!-- See https://checkstyle.org/config_sizes.html -->
<module name="MethodLength"/>
<module name="ParameterNumber"/>

<!-- 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"/>
<module name="HiddenField"/>
<module name="IllegalInstantiation"/>
<module name="InnerAssignment"/>
<module name="MissingSwitchDefault"/>
<module name="MultipleVariableDeclarations"/>
<module name="SimplifyBooleanExpression"/>
<module name="SimplifyBooleanReturn"/>

<!-- 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"/>

<!-- Miscellaneous other checks. -->
<!-- See https://checkstyle.org/config_misc.html -->
<module name="ArrayTypeStyle"/>
<module name="FinalParameters"/>
<module name="UpperEll"/>

<!-- https://checkstyle.org/config_filters.html#SuppressionXpathFilter -->
<module name="SuppressionXpathFilter">
<property name="file" value="${org.checkstyle.sun.suppressionxpathfilter.config}" default="checkstyle-xpath-suppressions.xml" />
<property name="optional" value="true"/>
</module>

</module>

</module>
16 changes: 16 additions & 0 deletions .github/linters/suppressed-java.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?xml version="1.0"?>

<!DOCTYPE suppressions PUBLIC
"-//Checkstyle//DTD SuppressionFilter Configuration 1.2//EN"
"https://checkstyle.org/dtds/suppressions_1_2.dtd"
>

<suppressions>
<suppress files="." checks="MagicNumber" /> <!-- Disabled because magic numbers are not generally abused and are more useful being allowed -->
<suppress files="." checks="TodoComment" /> <!-- We shouldn't have to be reminded about todo's on every linter iteration, we have IDE"s that do that! -->
<suppress files="." checks="JavadocVariable" /> <!-- Disabled as we don't need comments on every single new variable -->
<suppress files="." checks="FileLength" /> <!-- These two are disabled as we don't care if line or file size is too big -->
<suppress files="." checks="LineLength" />
<suppress files="." checks="Header" /> <!-- Disabled as we don't use headers in our project for the test files -->
<suppress files="." checks="FileTabCharacter" /> <!-- Disabled as it generally doesn't matter if tabs are disabled or not -->
</suppressions>
4 changes: 3 additions & 1 deletion .github/workflows/linter.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,6 @@ jobs:
VALIDATE_ALL_CODEBASE: false
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
# Markdown lint complains about the issue templates
FILTER_REGEX_EXCLUDE: .github/workflows/ISSUE_TEMPLATE/*
FILTER_REGEX_EXCLUDE: .github/ISSUE_TEMPLATE/*
# TODO: Disable the perl linter as there are lots of errors that are not really understood, see https://github.com/adoptium/temurin-build/issues/2612
VALIDATE_PERL: false
Loading

0 comments on commit 0d358c4

Please sign in to comment.