Skip to content

Commit

Permalink
fix: add missing fix mode options and test cases (#5987)
Browse files Browse the repository at this point in the history
- Add missing fix mode options for: CLANG_FORMAT, ENV,
  GOOGLE_JAVA_FORMAT, NATURAL_LANGUAGE, PYTHON_ISORT, RUST_CLIPPY.
- Refactor linter tests to make them shorter because there's no need to
  have big test files.
- Refactor 'bad' linter tests for linters that support fix mode so they
  contain only automatically fixable issues. This is needed to avoid
  adding another set of 'bad' linter tests for fix mode.
- Provide configuration files for linters that support fix mode and for
  which the default configuration is not suitable to enable fix mode:
  ansible-lint, ESLint, golangci-lint.
- Add a test case for linter commands options for linters that support
  fix mode, to ensure that fix mode and check-only mode options have
  been defined.
- Refactor the fix mode test to check if linters actually applied
  modifications to files.
- Update documentation about adding test cases for linters that support
  fix mode.
- Don't exit with a fatal error if VALIDATE_xxx is false when testing
  fix mode because not all linters support fix mode. To enable this, set
  the new FIX_MODE_TEST_CASE_RUN variable to true.
  • Loading branch information
ferrarimarco authored Aug 12, 2024
1 parent ea16cd9 commit 91dc6d7
Show file tree
Hide file tree
Showing 41 changed files with 594 additions and 711 deletions.
44 changes: 44 additions & 0 deletions .github/linters/.eslintrc.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
---
env:
browser: true
es6: true
jest: true
node: true

extends:
- "eslint:recommended"

ignorePatterns:
- "!.*"
- "**/node_modules/.*"

parser: '@typescript-eslint/parser'

plugins:
- '@typescript-eslint'

# Don't set the jsonSyntax parser option for JSON, JSON5, and JSONC
# so we can use eslint-plugin-jsonc to automatically fix issues
# in tests, otherwise ESLint reports parsing errors and stops
overrides:
- files:
- "*.json"
extends:
- plugin:jsonc/recommended-with-json
parser: jsonc-eslint-parser
- files:
- "*.jsonc"
extends:
- plugin:jsonc/recommended-with-jsonc
parser: jsonc-eslint-parser
- files:
- "*.json5"
extends:
- plugin:jsonc/recommended-with-json5
parser: jsonc-eslint-parser
- files:
- "*.jsx"
- "*.tsx"
extends:
- plugin:react/recommended
...
7 changes: 7 additions & 0 deletions .github/linters/.golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
# This file is only used in tests
# TODO: move in a dedicated directory in test/linters-config
linters:
enable:
- gofmt
...
1 change: 1 addition & 0 deletions .github/linters/.jscpd.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"**/test/linters/typescript_es/**",
"**/test/linters/typescript_prettier/**",
"**/test/linters/typescript_standard/**",
"**/test/linters-config/**",
"**/github_conf/**",
"**/workflows/cd.yml",
"**/workflows/ci.yml"
Expand Down
15 changes: 11 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
all: info docker test ## Run all targets.

.PHONY: test
test: info validate-container-image-labels docker-build-check docker-dev-container-build-check test-lib inspec lint-codebase fix-codebase test-default-config-files test-actions-runner-debug test-actions-steps-debug test-runner-debug test-find lint-subset-files test-custom-ssl-cert test-non-default-workdir test-git-flags test-non-default-home-directory test-git-initial-commit test-git-merge-commit-push test-log-level test-use-find-and-ignore-gitignored-files test-linters-expect-failure-log-level-notice test-bash-exec-library-expect-success test-bash-exec-library-expect-failure test-save-super-linter-output test-save-super-linter-output-custom-path test-save-super-linter-custom-summary test-linters test-linters-fix-mode-expect-success ## Run the test suite
test: info validate-container-image-labels docker-build-check docker-dev-container-build-check test-lib inspec lint-codebase fix-codebase test-default-config-files test-actions-runner-debug test-actions-steps-debug test-runner-debug test-find lint-subset-files test-custom-ssl-cert test-non-default-workdir test-git-flags test-non-default-home-directory test-git-initial-commit test-git-merge-commit-push test-log-level test-use-find-and-ignore-gitignored-files test-linters-expect-failure-log-level-notice test-bash-exec-library-expect-success test-bash-exec-library-expect-failure test-save-super-linter-output test-save-super-linter-output-custom-path test-save-super-linter-custom-summary test-linters test-linters-fix-mode ## Run the test suite

# if this session isn't interactive, then we don't want to allocate a
# TTY, which would fail, but if it is interactive, we do want to attach
Expand Down Expand Up @@ -318,6 +318,14 @@ test-globals-languages: ## Test globals/languages.sh
--entrypoint /tmp/lint/test/lib/globalsLanguagesTest.sh \
$(SUPER_LINTER_TEST_CONTAINER_URL)

.PHONY: test-globals-linter-command-options
test-globals-linter-command-options: ## Test globals/LinterCommandsOptions.sh
docker run \
-v "$(CURDIR):/tmp/lint" \
-w /tmp/lint \
--entrypoint /tmp/lint/test/lib/globalsLinterCommandsOptionsTest.sh \
$(SUPER_LINTER_TEST_CONTAINER_URL)

.PHONY: test-linter-rules
test-linter-rules: ## Test linterRules.sh
docker run \
Expand Down Expand Up @@ -416,14 +424,13 @@ test-non-default-home-directory: ## Test a non-default HOME directory
"run_test_cases_non_default_home" \
"$(IMAGE)"

.PHONY: test-linters-fix-mode-expect-success
test-linters-fix-mode-expect-success: ## Run the linters test suite (fix mode) expecting successes
.PHONY: test-linters-fix-mode
test-linters-fix-mode: ## Run the linters test suite (fix mode)
$(CURDIR)/test/run-super-linter-tests.sh \
$(SUPER_LINTER_TEST_CONTAINER_URL) \
"run_test_case_fix_mode" \
"$(IMAGE)"


.PHONY: test-linters
test-linters: test-linters-expect-success test-linters-expect-failure ## Run the linters test suite

Expand Down
16 changes: 15 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ You can configure Super-linter using the following environment variables:
| **FIX_RUST_2015** | `false` | Option to enable fix mode for `RUST_2015`. |
| **FIX_RUST_2018** | `false` | Option to enable fix mode for `RUST_2018`. |
| **FIX_RUST_2021** | `false` | Option to enable fix mode for `RUST_2021`. |
| **FIX_RUST_CLIPPY** | `false` | Option to enable fix mode for `RUST_CLIPPY`. |
| **FIX_RUST_CLIPPY** | `false` | Option to enable fix mode for `RUST_CLIPPY`. When `FIX_RUST_CLIPPY` is `true`, Clippy is allowed to fix issues in the workspace even if there are unstaged and staged changes in the workspace. |
| **FIX_SCALAFMT** | `false` | Option to enable fix mode for `SCALAFMT`. |
| **FIX_SHELL_SHFMT** | `false` | Option to enable fix mode for `SHELL_SHFMT`. |
| **FIX_SNAKEMAKE_SNAKEFMT** | `false` | Option to enable fix mode for `SNAKEMAKE_SNAKEFMT`. |
Expand Down Expand Up @@ -421,6 +421,20 @@ Super-linter supports the following locations to deliver fixes:
- If you're running Super-linter locally, you can commit the changes as you
would with any other change in your working directory.

### Fix mode for ansible-lint

ansible-lint requires that the `yaml` rule is enabled to for the ansible-lint
fix mode to work. The default ansible-lint configuration that Super-linter ships
disables the `yaml` rule because it might not be compatible with yamllint. If
you need to enable the ansible-lint fix mode, provide an ansible-lint
configuration that doesn't ignore the `yaml` rule.

### Fix mode file and directory ownership

When fix mode is enabled, some linters and formatters don't maintain the
original file or directory ownership, and use the user that Super-linter uses
to run the linter or formatter.

## Configure linters

Super-linter provides default configurations for some linters in the [`TEMPLATES/`](TEMPLATES/)
Expand Down
11 changes: 8 additions & 3 deletions docs/add-new-linter.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,16 @@ new tool, it should include:
- `README.md`
- Provide test cases:

1. Create the `test/linters/<LANGUGAGE>` directory.
1. Create the `test/linters/<LANGUAGE_NAME>` directory.
2. Provide at least one test case with a file that is supposed to pass validation,
with the right file extension if needed: `test/linters/<LANGUAGE>/<name-of-tool>-good`
with the right file extension if needed: `test/linters/<LANGUAGE_NAME>/<name-of-tool>-good`
3. Provide at least one test case with a file that is supposed to fail validation,
with the right file extension if needed: `test/linters/<LANGUAGE>/<name-of-tool>-bad`
with the right file extension if needed: `test/linters/<LANGUAGE_NAME>/<name-of-tool>-bad`.
If the linter supports fix mode, the test case supposed to fail validation
should only contain violations that the fix mode can automatically fix.
Avoid test cases that fail only because of syntax errors, when possible.
4. If the linter supports check-only mode or fix mode, add the `<LANGUGAGE>`
to the `LANGUAGES_WITH_FIX_MODE` array in `test/testUtils.sh`

- Update the test suite to check for installed packages, the commands that your new tool needs in the `PATH`, and the expected version command:

Expand Down
1 change: 1 addition & 0 deletions lib/functions/validation.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ function ValidateBooleanConfigurationVariables() {
ValidateBooleanVariable "DISABLE_ERRORS" "${DISABLE_ERRORS}"
ValidateBooleanVariable "ENABLE_GITHUB_ACTIONS_GROUP_TITLE" "${ENABLE_GITHUB_ACTIONS_GROUP_TITLE}"
ValidateBooleanVariable "ENABLE_GITHUB_ACTIONS_STEP_SUMMARY" "${ENABLE_GITHUB_ACTIONS_STEP_SUMMARY}"
ValidateBooleanVariable "FIX_MODE_TEST_CASE_RUN" "${FIX_MODE_TEST_CASE_RUN}"
ValidateBooleanVariable "IGNORE_GENERATED_FILES" "${IGNORE_GENERATED_FILES}"
ValidateBooleanVariable "IGNORE_GITIGNORED_FILES" "${IGNORE_GITIGNORED_FILES}"
ValidateBooleanVariable "LOG_DEBUG" "${LOG_DEBUG}"
Expand Down
7 changes: 6 additions & 1 deletion lib/functions/worker.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@ function LintCodebase() {
unset -n VALIDATE_LANGUAGE
return 0
else
fatal "Don't disable any validation when running in test mode. VALIDATE_${FILE_TYPE} is set to: ${VALIDATE_LANGUAGE}. Set it to: true"
if [[ "${FIX_MODE_TEST_CASE_RUN}" == "true" ]]; then
debug "Don't fail the test even if VALIDATE_${FILE_TYPE} is set to ${VALIDATE_LANGUAGE} because ${FILE_TYPE} might not support fix mode"
return 0
else
fatal "Don't disable any validation when running in test mode. VALIDATE_${FILE_TYPE} is set to: ${VALIDATE_LANGUAGE}. Set it to: true"
fi
fi
fi

Expand Down
8 changes: 6 additions & 2 deletions lib/globals/linterCommandsOptions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,12 @@ STANDARD_FIX_MODE_OPTIONS=(--fix)
# Define configuration options to enable "fix mode".
# Not all linters and formatters support this.
ANSIBLE_FIX_MODE_OPTIONS=(--fix)
CLANG_FORMAT_FIX_MODE_OPTIONS=(-i)
CSS_FIX_MODE_OPTIONS=(--fix)
ENV_FIX_MODE_OPTIONS=(fix)
ENV_FIX_MODE_OPTIONS=(fix --no-backup)
GO_FIX_MODE_OPTIONS=("${GOLANGCI_LINT_FIX_MODE_OPTIONS[@]}")
GO_MODULES_FIX_MODE_OPTIONS=("${GOLANGCI_LINT_FIX_MODE_OPTIONS[@]}")
GOOGLE_JAVA_FORMAT_FIX_MODE_OPTIONS=(--replace)
GROOVY_FIX_MODE_OPTIONS=(--fix)
JAVASCRIPT_ES_FIX_MODE_OPTIONS=("${ESLINT_FIX_MODE_OPTIONS[@]}")
JAVASCRIPT_PRETTIER_FIX_MODE_OPTIONS=("${PRETTIER_FIX_MODE_OPTIONS[@]}")
Expand All @@ -47,11 +49,13 @@ JSON_FIX_MODE_OPTIONS=("${ESLINT_FIX_MODE_OPTIONS[@]}")
JSONC_FIX_MODE_OPTIONS=("${ESLINT_FIX_MODE_OPTIONS[@]}")
JSX_FIX_MODE_OPTIONS=("${ESLINT_FIX_MODE_OPTIONS[@]}")
MARKDOWN_FIX_MODE_OPTIONS=(--fix)
NATURAL_LANGUAGE_FIX_MODE_OPTIONS=(--fix)
POWERSHELL_FIX_MODE_OPTIONS=(-Fix)
PROTOBUF_FIX_MODE_OPTIONS=(-fix)
PYTHON_ISORT_FIX_MODE_OPTIONS=(--overwrite-in-place)
PYTHON_RUFF_FIX_MODE_OPTIONS=(--fix)
RUBY_FIX_MODE_OPTIONS=(--autocorrect)
RUST_CLIPPY_FIX_MODE_OPTIONS=(--fix)
RUST_CLIPPY_FIX_MODE_OPTIONS=(--fix --allow-dirty --allow-staged)
SHELL_SHFMT_FIX_MODE_OPTIONS=(--write)
SQLFLUFF_FIX_MODE_OPTIONS=(fix)
TSX_FIX_MODE_OPTIONS=("${ESLINT_FIX_MODE_OPTIONS[@]}")
Expand Down
4 changes: 4 additions & 0 deletions lib/linter.sh
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ declare -l TEST_CASE_RUN
TEST_CASE_RUN="${TEST_CASE_RUN:-"false"}"
export TEST_CASE_RUN

declare -l FIX_MODE_TEST_CASE_RUN
FIX_MODE_TEST_CASE_RUN="${FIX_MODE_TEST_CASE_RUN:-"false"}"
export FIX_MODE_TEST_CASE_RUN

# We want a lowercase value
declare -l USE_FIND_ALGORITHM
USE_FIND_ALGORITHM="${USE_FIND_ALGORITHM:-false}"
Expand Down
44 changes: 44 additions & 0 deletions test/lib/globalsLinterCommandsOptionsTest.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#!/usr/bin/env bash

set -o errexit
set -o nounset
set -o pipefail

# Default log level
# shellcheck disable=SC2034
LOG_LEVEL="DEBUG"

# shellcheck source=/dev/null
source "lib/functions/log.sh"

# shellcheck source=/dev/null
source "test/testUtils.sh"

# The sqlfluff command needs this, but we don't want to make this test
# dependant on other files
# shellcheck disable=SC2034
SQLFLUFF_LINTER_RULES="SQLFLUFF_LINTER_RULES"

# shellcheck source=/dev/null
source "lib/globals/linterCommandsOptions.sh"

LanguagesWithFixModeTest() {
local FUNCTION_NAME
FUNCTION_NAME="${FUNCNAME[0]}"
info "${FUNCTION_NAME} start"

for LANGUAGE in "${LANGUAGES_WITH_FIX_MODE[@]}"; do
local FIX_MODE_OPTIONS_VARIABLE_NAME="${LANGUAGE}_FIX_MODE_OPTIONS"
local CHECK_ONLY_MODE_OPTIONS_VARIABLE_NAME="${LANGUAGE}_CHECK_ONLY_MODE_OPTIONS"
if [[ -v "${FIX_MODE_OPTIONS_VARIABLE_NAME}" ]] ||
[[ -v "${CHECK_ONLY_MODE_OPTIONS_VARIABLE_NAME}" ]]; then
debug "${LANGUAGE} has check-only mode or fix mode options as expected"
else
fatal "${LANGUAGE} is in the list of languages that support fix mode, but neither check-only mode, nor fix mode options were found"
fi
done

notice "${FUNCTION_NAME} PASS"
}

LanguagesWithFixModeTest
3 changes: 1 addition & 2 deletions test/lib/linterCommandsTest.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,12 @@ BASH_EXEC_IGNORE_LIBRARIES="false"
GITHUB_WORKSPACE="$(pwd)"
# shellcheck disable=SC2034
IGNORE_GITIGNORED_FILES="false"
LINTER_RULES_PATH="TEMPLATES"
# shellcheck disable=SC2034
TYPESCRIPT_STANDARD_TSCONFIG_FILE=".github/linters/tsconfig.json"
# shellcheck disable=SC2034
YAML_ERROR_ON_WARNING="false"
for LANGUAGE in "${LANGUAGE_ARRAY_FOR_LINTER_RULES[@]}"; do
GetLinterRules "${LANGUAGE}" "${LINTER_RULES_PATH}"
GetLinterRules "${LANGUAGE}" "TEMPLATES"
done
ValidateValidationVariables

Expand Down
7 changes: 6 additions & 1 deletion test/lib/linterRulesTest.sh
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,12 @@ function GetLinterRulesTest() {
GetLinterRules "${LANGUAGE}" "${DEFAULT_RULES_LOCATION}"
done

local EXPECTED_TEST_LANGUAGE_LINTER_RULES="${DEFAULT_RULES_LOCATION}/${TEST_LANGUAGE_FILE_NAME}"
local EXPECTED_TEST_LANGUAGE_LINTER_RULES="${GITHUB_WORKSPACE}"
if [[ -n "${LINTER_RULES_PATH:-}" ]]; then
EXPECTED_TEST_LANGUAGE_LINTER_RULES="${EXPECTED_TEST_LANGUAGE_LINTER_RULES}/${LINTER_RULES_PATH}"
fi
EXPECTED_TEST_LANGUAGE_LINTER_RULES="${EXPECTED_TEST_LANGUAGE_LINTER_RULES}/${TEST_LANGUAGE_FILE_NAME}"

if [[ "${TEST_LANGUAGE_LINTER_RULES}" == "${EXPECTED_TEST_LANGUAGE_LINTER_RULES}" ]]; then
debug "TEST_LANGUAGE_LINTER_RULES (${TEST_LANGUAGE_LINTER_RULES}) matches the expected value (${EXPECTED_TEST_LANGUAGE_LINTER_RULES})"
else
Expand Down
9 changes: 9 additions & 0 deletions test/linters-config/fix-mode/.ansible-lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
# Customize the ansible-lint configuration file because fix mode needs the
# yaml rule not to be disabled, but we disable the yaml rule in the default
# ansible-lint configuration
parseable: true
quiet: true
use_default_rules: true
verbosity: 1
...
44 changes: 44 additions & 0 deletions test/linters-config/fix-mode/.eslintrc.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
---
env:
browser: true
es6: true
jest: true
node: true

extends:
- "eslint:recommended"

ignorePatterns:
- "!.*"
- "**/node_modules/.*"

parser: '@typescript-eslint/parser'

plugins:
- '@typescript-eslint'

# Don't set the jsonSyntax parser option for JSON, JSON5, and JSONC
# so we can use eslint-plugin-jsonc to automatically fix issues
# in tests, otherwise ESLint reports parsing errors and stops
overrides:
- files:
- "*.json"
extends:
- plugin:jsonc/recommended-with-json
parser: jsonc-eslint-parser
- files:
- "*.jsonc"
extends:
- plugin:jsonc/recommended-with-jsonc
parser: jsonc-eslint-parser
- files:
- "*.json5"
extends:
- plugin:jsonc/recommended-with-json5
parser: jsonc-eslint-parser
- files:
- "*.jsx"
- "*.tsx"
extends:
- plugin:react/recommended
...
5 changes: 5 additions & 0 deletions test/linters-config/fix-mode/.golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
linters:
enable:
- gofmt
...
17 changes: 10 additions & 7 deletions test/linters/ansible/bad/playbooks/ansible_bad_1.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
---
- name: Remove temp files
become: true
file:
path: "{{ item }}"
state: absent
with_items:
- "/tmp/test-1"
- name: Test playbook
hosts: all
tasks:
- name: Remove temp files
become: true
file:
path: "{{ item }}"
state: absent
with_items:
- "/tmp/test-1"
2 changes: 1 addition & 1 deletion test/linters/ansible/good/playbooks/ansible_good_1.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@
path: "{{ item }}"
state: absent
with_items:
- "/tmp/test-1"
- /tmp/test-1
8 changes: 6 additions & 2 deletions test/linters/go/golang_bad_01.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
if len(in) == 0 {
return "", fmt.Errorf("Input is empty")
package main

import "fmt"

func main() {
fmt.Println("hello world")
}
Loading

0 comments on commit 91dc6d7

Please sign in to comment.