Skip to content

Commit 51280ab

Browse files
committed
bug(#334): add three registry suggestions to --fix
Register node => fix builders in src/fixers.js for three declarative rules, each a suggestion applied only under --fix-suggestions: output-method-xml (switch method="xml" to "html"), missing-version-in-stylesheet (insert version="1.0" after the element name), and mode-or-priority-without-match (delete the orphan attribute when exactly one of mode/priority is present, else leave the defect fix-less). The per-file linter now skips attaching a fix when the builder returns null. No new linter or category move — a declarative rule carries a fix through the registry.
1 parent 49d88b6 commit 51280ab

12 files changed

Lines changed: 172 additions & 13 deletions

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@ publication date only; detailed notes begin with the Unreleased section.
99

1010
## Unreleased
1111

12+
- Add three more `--fix-suggestions`, each a `src/fixers.js` registry entry:
13+
`output-method-xml` (switch `method="xml"` to `"html"`),
14+
`missing-version-in-stylesheet` (declare `version="1.0"`), and
15+
`mode-or-priority-without-match` (delete the orphan attribute, when exactly
16+
one of `mode`/`priority` is present) (#334).
17+
1218
- Add a suggestion tier to `--fix`. A fix marked `suggestion` is opinionated —
1319
it changes behavior, removes code, or is one of several valid corrections —
1420
so `--fix` leaves it and only the new `--fix-suggestions` applies it. A

CLAUDE.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,9 @@ Configuration by users: a `.xslint.yml` file (discovered by walking up from the
137137

138138
Command-line flags override the file; the file overrides the built-in defaults. Resolution lives in `src/config.js` (which also exposes the config's `base` directory); `src/xslint.js` expands each rule pattern against the check names, folds `off` rules into the suppression list, filters excluded files against `base`, applies severity overrides to the collected defects, and resolves the effective `max-warnings`/`log-level`/`quiet`.
139139

140-
Fixing by users: `xslint --fix` rewrites the mechanically-fixable defects in place (`--fix-dry-run` reports the same result without writing any file). A defect is fixable when it carries a `fix: {line, col, value, replacement, suggestion?}` — `xpath-format-linter` attaches one to each `redundant-whitespace` defect, `xpath-axis-linter` to each `unabbreviated-axis` defect, `namespace-linter` to each `redundant-namespace-declarations` defect (its `value` is the reconstructed ` xmlns:prefix="uri"` text, so the verify step skips a single-quoted or oddly spaced declaration rather than deleting the wrong span), and `node-set-linter` to each `use-node-set-extension` defect (a single span from `prefix:node-set(` through its matching `)`, replaced by the inner argument). A *declarative* xpath rule can carry a fix too, without becoming a code-based linter: `src/fixers.js` maps a check name to a `node => fix` builder, and `xpath-linter` attaches the fix that builder returns to each defect it finds for that check — that is how `using-disable-output-escaping` gets its fix. Attribute-deleting fixes (the namespace declaration, the `disable-output-escaping` attribute) share `deletion(attribute)` in `src/fixes.js`. `src/fixer.js` maps each fix's position to a source offset and applies it *only* when the span still holds the exact `value` (a shifted offset — an entity ahead of the run, an already-edited file — is skipped, never corrupting the source), applying a file's fixes from the end backwards so earlier offsets stay valid, and returns the rewritten content per changed file plus the defects it applied. `src/xslint.js` writes the changed files (unless `--fix-dry-run`) and drops the applied defects from the report, so the exit code reflects only what remains.
140+
Fixing by users: `xslint --fix` rewrites the mechanically-fixable defects in place (`--fix-dry-run` reports the same result without writing any file). A defect is fixable when it carries a `fix: {line, col, value, replacement, suggestion?}` — `xpath-format-linter` attaches one to each `redundant-whitespace` defect, `xpath-axis-linter` to each `unabbreviated-axis` defect, `namespace-linter` to each `redundant-namespace-declarations` defect (its `value` is the reconstructed ` xmlns:prefix="uri"` text, so the verify step skips a single-quoted or oddly spaced declaration rather than deleting the wrong span), and `node-set-linter` to each `use-node-set-extension` defect (a single span from `prefix:node-set(` through its matching `)`, replaced by the inner argument). A *declarative* xpath rule can carry a fix too, without becoming a code-based linter: `src/fixers.js` maps a check name to a `node => fix` builder, and `xpath-linter` attaches the fix that builder returns to each defect it finds for that check (a builder returns null when it cannot resolve the defect with one edit, and the linter then leaves the defect fix-less). The registry drives four suggestions — `using-disable-output-escaping` and `mode-or-priority-without-match` (delete an attribute), `output-method-xml` (`method="xml"`→`"html"`), and `missing-version-in-stylesheet` (insert `version="1.0"` after the element name). Attribute-deleting fixes (the namespace declaration, the `disable-output-escaping` and orphan `mode`/`priority` attributes) share `deletion(attribute)` in `src/fixes.js`. `src/fixer.js` maps each fix's position to a source offset and applies it *only* when the span still holds the exact `value` (a shifted offset — an entity ahead of the run, an already-edited file — is skipped, never corrupting the source), applying a file's fixes from the end backwards so earlier offsets stay valid, and returns the rewritten content per changed file plus the defects it applied. `src/xslint.js` writes the changed files (unless `--fix-dry-run`) and drops the applied defects from the report, so the exit code reflects only what remains.
141141

142-
Fixes come in two tiers, set by the `suggestion` flag on the `fix`. A plain fix is *safe* — deterministic and semantics-preserving — and `--fix` applies it. A fix with `suggestion: true` is *opinionated* — it changes behavior, removes code, or is one of several valid corrections — so `--fix` leaves it and only `--fix-suggestions` applies it (`fixer.js` takes a `suggestions` flag and gates on it). `using-disable-output-escaping` is the first suggestion: deleting the attribute changes how output is escaped. When neither flag is passed, `xslint.js` counts the fixable defects and logs how many `--fix` would fix and how many more `--fix-suggestions` would. The fix engine is check-agnostic — the `fix` shape is the whole contract — so a new fixer attaches a `fix` (marking it a suggestion when it should be opt-in) and needs no engine change; the structural fixes (removing an unused variable, merging nested `xsl:if`) wait on the full-fidelity parser (#228) for exact per-node source spans.
142+
Fixes come in two tiers, set by the `suggestion` flag on the `fix`. A plain fix is *safe* — deterministic and semantics-preserving — and `--fix` applies it. A fix with `suggestion: true` is *opinionated* — it changes behavior, removes code, or is one of several valid corrections — so `--fix` leaves it and only `--fix-suggestions` applies it (`fixer.js` takes a `suggestions` flag and gates on it). Every fix in `src/fixers.js` is a suggestion so far — deleting `disable-output-escaping` changes escaping, switching the output method changes serialization, the declared version is a guess, and dropping `mode`/`priority` is one of two corrections. When neither flag is passed, `xslint.js` counts the fixable defects and logs how many `--fix` would fix and how many more `--fix-suggestions` would. The fix engine is check-agnostic — the `fix` shape is the whole contract — so a new fixer attaches a `fix` (marking it a suggestion when it should be opt-in) and needs no engine change; the structural fixes (removing an unused variable, merging nested `xsl:if`) wait on the full-fidelity parser (#228) for exact per-node source spans.
143143

144144
## Keeping Docs in Sync
145145

@@ -162,7 +162,7 @@ A change that leaves any of these describing the old behavior is not done.
162162
| `src/xsl-validator.js` | Builds the corpus from raw sources; reports each stylesheet that is not well-formed XML and leaves it out |
163163
| `src/xpath-validator.js` | Splits each corpus expression into the valid ones (kept for the expression linters) and the malformed ones (reported) |
164164
| `src/xpath-linter.js` | Loads `checks/xpath/*.yaml`, applies per-file XPath rules, and attaches any `src/fixers.js` fix to the defect |
165-
| `src/fixers.js` | Maps a declarative check name to a `node => fix` builder, so an xpath rule can carry a fix (e.g. the `using-disable-output-escaping` suggestion) |
165+
| `src/fixers.js` | Maps a declarative check name to a `node => fix` builder, so an xpath rule can carry a fix the suggestions for `using-disable-output-escaping`, `output-method-xml`, `missing-version-in-stylesheet`, `mode-or-priority-without-match` |
166166
| `src/fixes.js` | Shared fix builders — `deletion(attribute)` reconstructs an attribute's ` name="value"` span for removal |
167167
| `src/corpus-linter.js` | Loads `checks/corpus/*.yaml`, applies cross-file rules over the corpus |
168168
| `src/xpath-axis-linter.js` | Tokenizes every XPath/pattern attribute in the corpus and flags each verbose axis (`unabbreviated-axis`) with a fix |

README.md

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -249,10 +249,20 @@ silently by `--fix`:
249249
xslint --fix-suggestions path/to/dir
250250
```
251251

252-
Today this covers `using-disable-output-escaping` — the attribute is removed,
253-
which changes how the output is escaped, so you opt in. Checks whose correction
254-
needs real judgment (a fresh name, a more specific path) stay report-only. A
255-
run without `--fix` reports how many defects each option would fix.
252+
Today this covers:
253+
254+
- `using-disable-output-escaping` — the attribute is removed, which changes how
255+
the output is escaped.
256+
- `output-method-xml` — `method="xml"` becomes `"html"` when the stylesheet
257+
emits HTML.
258+
- `missing-version-in-stylesheet` — `version="1.0"` is declared (a guess you may
259+
want to change).
260+
- `mode-or-priority-without-match` — the orphan `mode` or `priority` attribute
261+
is removed.
262+
263+
Checks whose correction needs real judgment (a fresh name, a more specific
264+
path) stay report-only. A run without `--fix` reports how many defects each
265+
option would fix.
256266

257267
Pass `--fix-dry-run` to see what would remain after fixing, without writing any
258268
file:

src/fixers.js

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@ const {deletion} = require('./fixes')
77

88
/**
99
* Fix for `using-disable-output-escaping`: delete the attribute. Removing it
10-
* changes how the output is escaped, so it is a suggestion — applied only under
11-
* `--fix-suggestions`, never silently by `--fix`.
10+
* changes how the output is escaped, so it is a suggestion.
1211
* @param {Element} node - The element carrying the attribute
1312
* @return {object} - The suggestion fix
1413
*/
@@ -19,14 +18,66 @@ const disableOutputEscaping = function(node) {
1918
}
2019
}
2120

21+
/**
22+
* Fix for `output-method-xml`: switch the method to `html`. It changes the
23+
* serialization, so it is a suggestion.
24+
* @param {Element} node - The `xsl:output` element
25+
* @return {object} - The suggestion fix
26+
*/
27+
const outputMethodXml = function(node) {
28+
const method = node.getAttributeNode('method')
29+
return {
30+
line: method.lineNumber,
31+
col: method.columnNumber + 1,
32+
value: 'xml',
33+
replacement: 'html',
34+
suggestion: true,
35+
}
36+
}
37+
38+
/**
39+
* Fix for `missing-version-in-stylesheet`: declare `version="1.0"` right after
40+
* the element name. The version is a guess, so it is a suggestion.
41+
* @param {Element} node - The `xsl:stylesheet` element
42+
* @return {object} - The suggestion fix
43+
*/
44+
const missingVersion = function(node) {
45+
return {
46+
line: node.lineNumber,
47+
col: node.columnNumber + node.nodeName.length + 1,
48+
value: '',
49+
replacement: ' version="1.0"',
50+
suggestion: true,
51+
}
52+
}
53+
54+
/**
55+
* Fix for `mode-or-priority-without-match`: delete the orphan attribute. It is
56+
* one of two corrections the rule offers (the other is adding `match`), so it
57+
* is a suggestion, and only when exactly one of `mode`/`priority` is present
58+
* can a single deletion resolve the defect — with both, there is no fix.
59+
* @param {Element} node - The `xsl:template` element
60+
* @return {?object} - The suggestion fix, or null
61+
*/
62+
const modeOrPriority = function(node) {
63+
const present = ['mode', 'priority'].filter((name) => node.hasAttribute(name))
64+
return present.length === 1 ?
65+
{...deletion(node.getAttributeNode(present[0])), suggestion: true} :
66+
null
67+
}
68+
2269
/**
2370
* Fix builders for declarative Xpath checks, keyed by check name. The per-file
2471
* linter attaches the fix a builder returns to the defect it found for that
25-
* check, so a rule stays declarative while still carrying a fix.
26-
* @type {{[check: string]: function(Node): object}}
72+
* check, so a rule stays declarative while still carrying a fix; a builder
73+
* returns null when it cannot resolve the defect with a single edit.
74+
* @type {{[check: string]: function(Node): ?object}}
2775
*/
2876
const FIXERS = {
2977
'using-disable-output-escaping': disableOutputEscaping,
78+
'output-method-xml': outputMethodXml,
79+
'missing-version-in-stylesheet': missingVersion,
80+
'mode-or-priority-without-match': modeOrPriority,
3081
}
3182

3283
module.exports = {

src/xpath-linter.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,9 @@ const lintByXpath = function(corpus, suppressions = []) {
6969
line: node.lineNumber,
7070
pos: node.columnNumber,
7171
}
72-
if (FIXERS[pack.name]) {
73-
defect.fix = FIXERS[pack.name](node)
72+
const fix = FIXERS[pack.name] && FIXERS[pack.name](node)
73+
if (fix) {
74+
defect.fix = fix
7475
}
7576
defects.push(defect)
7677
}

test/fixer.test.js

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,31 @@ describe('fixer', function() {
101101
fixture('using-disable-output-escaping.fixed.xsl'),
102102
)
103103
})
104+
it('should switch an xml output method to html with --fix-suggestions',
105+
function() {
106+
const file = scratch(fixture('output-method-xml.xsl'))
107+
runXslint(['--fix-suggestions', file])
108+
assert.equal(
109+
fs.readFileSync(file, 'utf-8'),
110+
fixture('output-method-xml.fixed.xsl'),
111+
)
112+
})
113+
it('should declare a missing version with --fix-suggestions', function() {
114+
const file = scratch(fixture('missing-version-in-stylesheet.xsl'))
115+
runXslint(['--fix-suggestions', file])
116+
assert.equal(
117+
fs.readFileSync(file, 'utf-8'),
118+
fixture('missing-version-in-stylesheet.fixed.xsl'),
119+
)
120+
})
121+
it('should drop an orphan mode with --fix-suggestions', function() {
122+
const file = scratch(fixture('mode-or-priority-without-match.xsl'))
123+
runXslint(['--fix-suggestions', file])
124+
assert.equal(
125+
fs.readFileSync(file, 'utf-8'),
126+
fixture('mode-or-priority-without-match.fixed.xsl'),
127+
)
128+
})
104129
it('should announce how many defects --fix would fix', function() {
105130
const file = scratch(dirty)
106131
assert.ok(xslintStreams([file]).stderr.includes('fixable with --fix'))
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
* SPDX-FileCopyrightText: Copyright (c) 2025-2026 Max Trunnikov
4+
* SPDX-License-Identifier: MIT
5+
-->
6+
<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
7+
<xsl:template match="/">
8+
<xsl:value-of select="."/>
9+
</xsl:template>
10+
</xsl:stylesheet>
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
* SPDX-FileCopyrightText: Copyright (c) 2025-2026 Max Trunnikov
4+
* SPDX-License-Identifier: MIT
5+
-->
6+
<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
7+
<xsl:template match="/">
8+
<xsl:value-of select="."/>
9+
</xsl:template>
10+
</xsl:stylesheet>
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
* SPDX-FileCopyrightText: Copyright (c) 2025-2026 Max Trunnikov
4+
* SPDX-License-Identifier: MIT
5+
-->
6+
<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform" version="2.0">
7+
<xsl:template name="city">
8+
<xsl:value-of select="."/>
9+
</xsl:template>
10+
</xsl:stylesheet>
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
* SPDX-FileCopyrightText: Copyright (c) 2025-2026 Max Trunnikov
4+
* SPDX-License-Identifier: MIT
5+
-->
6+
<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform" version="2.0">
7+
<xsl:template name="city" mode="Bryansk">
8+
<xsl:value-of select="."/>
9+
</xsl:template>
10+
</xsl:stylesheet>

0 commit comments

Comments
 (0)