Skip to content

Tags: UltraBob/ddev-drupal-code-quality

Tags

v1.0.6

Toggle v1.0.6's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Node tooling at project root, installer UX improvements, scan scope f…

…ixes (#32)

* Add shared root-only Node toolchain resolver

Introduce commands/helpers/node-toolchain.sh with resolve_node_tool(),
a single function that looks up Node tool binaries exclusively in the
project root node_modules. All Node wrappers will source this helper
instead of duplicating resolution logic.

Register the helper in install.yaml so DDEV copies it into
.ddev/commands/helpers/ on add-on install.

* Route stylelint wrappers through shared Node resolver

Replace CORE_STYLELINT/ROOT_STYLELINT variables and the if/elif/else
fallback chain with resolve_node_tool(). Remove the core/node_modules
tar exclude from stylelint-fix preview mode, replacing it with a
generic docroot-relative nested node_modules exclude. Update error
messages to reference the project root and the installer.

* Route eslint wrappers through shared Node resolver

Replace CORE_ESLINT/ROOT_ESLINT, ESLINT_TOOLCHAIN/prefer_root logic,
and the core fallback in resolve_plugins_dir() with resolve_node_tool().
Remove the core/node_modules tar exclude from eslint-fix preview mode,
replacing it with a generic docroot-relative nested exclude. Update
error messages to reference the project root and the installer.

* Route prettier wrappers through shared Node resolver

Replace CORE_PRETTIER/ROOT_PRETTIER variables and the core fallback
chain with resolve_node_tool(). Rename PRETTIER_BIN to TOOLCHAIN_BIN
for consistency with the helper contract. Remove the core/node_modules
tar exclude from prettier-fix preview mode, replacing it with a generic
docroot-relative nested exclude. Update error messages to reference the
project root and the installer.

* Route cspell wrappers through shared Node resolver

Replace CORE_CSPELL/ROOT_CSPELL variables and the core fallback chain
with resolve_node_tool(). Rename CSPELL_BIN to TOOLCHAIN_BIN for
consistency with the helper contract. Update error messages to reference
the project root and the installer.

* Remove core/node_modules fallback from installer and tests

Strip the two core/node_modules paths from node_toolchain_present() so
it only checks the project root. Add legacy core-only detection: when
the root toolchain is absent but core has eslint, non-interactive mode
auto-installs at root and interactive mode falls through to the prompt.

Rename DOCROOT_CORE to DOCROOT_COREDIR in the package.json derivation
helpers to avoid false positives on the DoD grep gate (the variable
points at Drupal's core/ source dir, not node_modules).

In tests: remove the web/core/node_modules early-return from
ensure_node_toolchain(), add an ESLint sentinel to the fake-core-
binaries regression fixture, and add an eslint --version assertion.

Update README to remove stale ESLINT_TOOLCHAIN=core/auto/root docs
and note the root-only toolchain policy.

* Simplify stylelint wrappers by delegating config and ignore handling to stylelint

Rewrite both stylelint and stylelint-fix as thin delegating wrappers
(~90 lines each, down from 400+). Native stylelint behavior now handles
config discovery (cosmiconfig walk), .stylelintignore auto-application,
plugin resolution, and SCSS detection — all previously reimplemented in
the wrapper scripts.

Wrapper responsibilities are now limited to: toolchain resolution via
the shared helper, host-path translation (map_path for IDE integration),
default-glob injection on bare invocation, and cd to PROJECT_ROOT.

This eliminates the CMD-array-reassignment bug class (issue #27) by
construction and fixes an unreported bug where stylelint.config.js
configs used by modern Drupal themes were silently skipped.

The --preview mode in stylelint-fix is removed; a deprecation warning
directs users to git diff as the standard review workflow.

* Narrow default scan to CSS and add SCSS setup hint

Default globs now only include **/*.css — the base config shipped by
DCQ is CSS-only, so including SCSS/Sass by default would cause parse
errors on projects that haven't opted into SCSS linting.

When SCSS/Sass files are detected during a bare invocation, a one-line
hint is emitted to stderr suggesting the stylelint-config-standard-scss
setup recipe. Users can still lint SCSS explicitly via glob arguments.

Also hardens --preview removal: the flag now exits with an error
instead of falling through to --fix.

* Fix SCSS hint to detect .scss files and skip dependency directories

The find expression had an operator precedence bug that only matched
.sass files (not .scss) due to implicit -print behavior with -o. Also
prune node_modules and vendor to avoid spurious hints triggered by
SCSS files in dependencies rather than user code.

* Update bats tests for minimal stylelint wrapper contract

Rewrite stylelint-related tests to assert the new wrapper behavior:
native config discovery and ignore-file handling instead of wrapper-
injected --config, --config-basedir, and --ignore-path flags.

- Merge two near-identical config-delegation tests into one
- Rewrite ignore-path regression tests (issue #27) to verify the flag
  injection mechanism no longer exists, making the bug structurally
  impossible
- Add new test for SCSS hint on default invocation
- Strengthen non-web docroot test with echo stub to verify path
  pass-through
- Update full-test preview section for --preview removal (exit 1)

37/37 pass (35 active, 2 skipped for DCQ_FULL_TESTS).
Full-test suite: 2/2 pass (fresh install + preview mode).

* Fix escaped command display in installer run_command output

The printf %q format was shell-escaping arguments in the "Running:" line,
producing unreadable output like "cd\ /var/www/html\ \&\&\ npm\ install".
Use %s instead to show commands as the user would type them.

* Improve SCSS hint wording and fix escaped installer output

- SCSS hint now shows how to add SCSS to default runs rather than
  suggesting explicit paths each time
- Fix printf %q escaping in run_command that produced unreadable
  "Running:" lines in installer output

* Include SCSS and Sass in default stylelint scan globs

Default globs now include **/*.scss and **/*.sass alongside **/*.css,
so bare `ddev stylelint` scans all style files. Users who configure
SCSS support (e.g. extending stylelint-config-standard-scss) no longer
need to pass explicit globs on every invocation.

Removes the SCSS hint and its find-based detection logic.

* Add dist, build, and playwright-report to default ignore lists

Generated output directories cause false positives and slow scans.
Adds **/dist/** and **/build/** as active exclusions and
**/playwright-report/** as a commented-out option with a note.

* Default stylelint globs to CSS-only with configurable override

The shipped .stylelintrc.json only supports CSS, so default scan globs
now match only **/*.css. Users can override via DCQ_STYLELINT_GLOBS in
web_environment to add SCSS/Sass/other patterns. Ships a new
config.drupal-code-quality.yaml with the commented default.

* Add SCSS detection and guided setup to installer

Detects .scss/.sass files during install and offers interactive SCSS
support setup. When accepted, writes SCSS globs to the add-on config
and shows remaining manual steps (package install, config edit) in the
install summary. Non-interactive installs print full guidance. Checks
both stylelint config and DDEV env to avoid re-prompting when SCSS is
already fully configured.

* Fix IDE settings merge losing Python exit codes

The merge_json_settings and merge_json_extensions functions used
`if ! python ...; then return 1; fi` which always returned 1 for any
non-zero Python exit, discarding the actual exit code. This meant
exit 3 (no changes needed) was misreported as a merge error, causing
"Unable to merge IDE settings/extensions" on projects where settings
were already up to date. Propagate the real exit code with `return $?`.

* Fix SCSS detection to check nested configs and respect recommended mode

SCSS detection now searches all .stylelintrc* files up to 4 levels deep
instead of only the project root, so themes with their own SCSS config
are properly detected as configured. Also gates the interactive SCSS
prompt behind recommended_mode=0 so it doesn't appear when the user
accepts recommended defaults.

* Handle composer require failure gracefully with retry/proceed/abort

When composer require fails during Phase 1 (e.g., security advisories
blocking resolution, allow-plugins not configured), the installer now
catches the failure instead of aborting. Users see the full composer
error output plus common causes, and get three options:
- retry (fix the issue in another terminal, then try again)
- proceed (continue with remaining phases without PHP dev tools)
- abort (stop the installer entirely)

In non-interactive mode, the installer auto-proceeds. The install
summary reflects the failure with a FIX action item in next steps.

Validated against all 11 projects that failed in UAT (24% of tested
projects). All now complete installation successfully.

* Add Node.js version check to prevent stylelint crashes on old Node

Stylelint crashes with a cryptic TypeError (findLastIndex) on Node.js 16,
which was found on 2 of 46 UAT projects (ccet, kcom3). Both had
nodejs_version: "16" in their DDEV config and no engines field in
package.json.

Installer changes:
- Detect nodejs_version from .ddev/config.yaml before Phase 3
- Prompt abort/skip when Node < 18 with actionable fix commands
- Offer to add engines.node >= 20.0 to existing package.json

Runtime wrapper changes:
- Add Node >= 18 check to stylelint and stylelint-fix wrappers
- Clear error message instead of cryptic findLastIndex crash

Documentation:
- Add Troubleshooting section to README covering the findLastIndex
  crash and the string formatter RangeError (known stylelint wontfix,
  workaround: --formatter compact)

Tests:
- Wrapper rejects Node < 18 with clear error
- Installer prompts abort/skip on old Node
- Installer offers engines.node for package.json without it

* Add curated DCQ package list and upstream drift detection

Introduce dcq-packages.json with the specific npm packages DCQ configs
require, replacing the previous approach of copying all core devDependencies.
The installer now resolves only these packages (with versions from core)
when creating project-root package.json files.

Add package list drift check to sync-upstream-configs.sh so curated
packages are validated against core on each sync.

* Exclude DDEV-generated files and playwright-report from default scans

Suppress false positives on fresh installs by excluding files that
are not user-authored code:

- settings.ddev.php excluded from PHPCS, PHPStan, and CSpell
- default.settings.php excluded from PHPStan (template, not user-edited)
- playwright-report/ excluded from all JS/CSS linters and CSpell
  (generated build output with minified code)

* Add stylelint config amendment with allowEmptyInput

Create .stylelintrc.dcq.json amendment file that adds allowEmptyInput: true
to the stylelint config during installation. This prevents stylelint from
failing with AllFilesIgnoredError on projects with no custom CSS files
(e.g., a fresh Drupal install).

Uses the config-amendments pattern consistent with phpstan.dcq.neon and
.phpcs.dcq.xml, keeping the upstream .stylelintrc.json untouched.

* Improve installer UX: accurate summary, reduced noise, config amendments

Address findings from UX audit of the installer:

- Summary reports actual tool state ("already present (skipped)") instead
  of claiming "NOT installed" when tools exist from a prior install
- Phase 1 emits "PHP dev tools already present" when nothing to install
- Suppress "Removed dcq-install.sh" implementation detail from output
- SCSS setup steps use letters (a-d) to avoid confusion with numbered
  next-steps list
- Add stylelint config merge via .stylelintrc.dcq.json amendment
- Exclude dcq-packages.json from project root copy (internal manifest)
- Clean up stale dcq-packages.json from prior installs
- Add "this may take several minutes" to npm install message
- Use ddev npm/yarn wrappers instead of ddev exec in install commands
- Use curated dcq-packages.json for dependency resolution

* Fix stylelint formatter crash on large projects

Use compact formatter for stylelint in ddev checks to prevent
RangeError crash when the table formatter tries to render thousands
of violations. The standalone ddev stylelint command keeps the default
table formatter for smaller projects.

Also fix the stylelint wrapper argument parser to properly handle
flags that take a value argument (--formatter, --custom-syntax, etc.)
without treating the value as a positional file path.

* Add tests for UX audit fixes and update stub package.json

Add test coverage for:
- dcq-packages.json not copied to project root
- allowEmptyInput merged into .stylelintrc.json
- settings.ddev.php excluded from PHPCS, PHPStan, CSpell
- playwright-report excluded from ignore files
- Summary says "already present" on re-install
- No "Removed dcq-install.sh" in output

Update stub package.json fixtures to include the full curated DCQ
package list matching dcq-packages.json.

* Exclude config/sync from CSpell and scope theme scanning to custom only

Address issue #30: default scans produce false positives from
non-user-authored content.

- CSpell: add config/sync to ignorePaths. Exported Drupal config YAML
  contains UUIDs and hash tokens that are not misspelled words.
- All tools: only scan themes/custom/, excluding non-custom themes like
  blank/ (ships with Drupal CMS outside contrib/). Applied consistently:
  - PHPCS: explicit <file> for themes/custom
  - PHPStan: already scoped to themes/custom
  - Stylelint, ESLint, Prettier: ignore themes/** then un-ignore themes/custom/**
  - CSpell: ignore themes/!(custom)/**

* Sync upstream CSpell words and fix Mutagen sync race in tests

- Apply upstream prepare-cspell.php change: add 'flagwords' and 'mkdocs'
  to CSpell word list (closes #28)
- Fix wait_for_container_path to flush Mutagen sync and check file has
  content (test -s) not just existence (test -r). Under high system load,
  file inodes appear before content syncs, causing ESLint and other tools
  to see empty files and report no errors.

* Fix theme ignore negation pattern across all tools

The previous pattern (**/themes/** + !**/themes/custom/**) did not work
in ESLint 8 or stylelint — the negation failed to un-ignore files,
causing custom theme code to be silently skipped.

Switch to **/themes/*/ + !**/themes/custom/ which ignores directory
entries one level under themes/ except custom/. Verified working in
ESLint 8, stylelint, and prettier.

Also remove debug instrumentation from test file.

* Fix test assertions for PHPCS scope change and new installer behavior

- Test 10: create fake vendor/bin tools so "already present" check works
  in test environments without real composer installs
- Test 11: narrow refute_output to "Removed" only — "dcq-install.sh"
  appears in DDEV's own file copy output
- Test 20: update docroot assertions for new PHPCS scan paths
  (modules, themes/custom, profiles, sites instead of whole docroot)

v1.0.5

Toggle v1.0.5's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Upstream config changes (#29)

* Set PHPCS default scope to docroot and test docroot merge

* Add upstream config sync script, detection accuracy tests, and volume cleanup

Add a maintainer script that fetches the latest config files from Drupal core
and GitLab CI templates, compares them against local assets, and optionally
applies updates. A weekly GitHub Actions workflow detects upstream drift and
opens an issue when changes are found.

Expand the full install test with detection accuracy assertions for every
wrapper command: true positives (tools catch known violations), true negatives
(clean code passes), and exclusion path tests (contrib code is not flagged).

Clean up Docker volumes (mariadb, mutagen, snapshots) in test teardown to
prevent accumulation across test runs.

* Fix cspell comparison to ignore arrays populated by prepare-cspell.php

The local .cspell.json asset has populated ignorePaths, dictionaries,
dictionaryDefinitions, and words arrays (expanded by prepare-cspell.php
at install time). Upstream has empty arrays. Strip these from both sides
before comparing so the script only flags structural changes to fields
like description, flagWords, and overrides.

Also revert accidental asset updates from test run.

* Update upstream config

* Ensure trailing newlines on assets for DDEV addon checker

The DDEV addon update checker requires files to end with a newline, but
some upstream files lack one. Add trailing newline to .eslintrc.jquery.json
and normalize fetched files in the sync script so the difference doesn't
cause false positives.

v1.0.4

Toggle v1.0.4's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
fix: handle npm install failures gracefully instead of crashing insta…

…ller (#24)

The installer crashed with exit status 1 when npm install failed during
Phase 3, because unguarded run_command calls allowed set -e to abort the
entire post-install action.

Changes:
- Wrap all npm/yarn install calls in error handlers so failures produce
  helpful retry messages instead of crashing the installer
- Replace mapfile with a bash 3.2-compatible while-read loop (macOS
  ships bash 3.2 which lacks mapfile, causing an unbound variable crash)
- Use test -s instead of test -f when waiting for .cspell.json in the
  container, so the wait detects empty files from partial filesystem sync
- Only emit "Node toolchain installed" when installation actually succeeds

Fixes #23

v1.0.3

Toggle v1.0.3's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Fix PHPStan excludes for `sites/*/files` descendants (#8)

v1.0.2

Toggle v1.0.2's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Improve cspell expansion execution and prettier-fix Sass scope (#5)

* Remove hidden --quiet flag from eslint wrapper

* Remove hidden --quiet flag from eslint-fix wrapper

* Make ESLint quiet default explicit and IDE-aligned

* Clarify ESLint quiet config for CLI and VS Code

* Improve cspell expansion execution and prettier-fix sass scope

v1.0.1

Toggle v1.0.1's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Update project title in README.md

v1.0.0

Toggle v1.0.0's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Set PHPCS default scope to docroot and test docroot merge (#2)

* Set PHPCS default scope to docroot and test docroot merge

* Document PHPCS default docroot scan scope

v0.9.5-beta

Toggle v0.9.5-beta's commit message
Add maintenance checker CI gate and mutagen sync metadata

v0.9.4-beta

Toggle v0.9.4-beta's commit message
Remove checks-full from install manifest and docs

v0.9.3-beta

Toggle v0.9.3-beta's commit message
Fix test assertions for preview mode after fix command refactor

The recent refactor of fix commands removed the summary output and the
test update commits missed correcting two assertions:

- Changed assertion from removed "ESLint-fix summary:" to actual
  "Running ESLint to report any remaining issues" output
- Fixed patch filename assertions to use correct names (_eslint.patch,
  _prettier.patch, _stylelint.patch) instead of incorrect names that
  were never generated by the commands