Skip to content

Commit 879e110

Browse files
fixing and simplifying logic for content exclusion to catch certain edge cases
1 parent 378f62d commit 879e110

File tree

3 files changed

+50
-94
lines changed

3 files changed

+50
-94
lines changed

scripts/prebuild/mdx-transforms/exclude-content/ast-utils.mjs

Lines changed: 38 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ function isCommentNode(node) {
2929
* Parse all directive blocks from AST in a single pass
3030
*
3131
* @param {Object} tree Remark AST
32-
* @returns {Array} Array of block objects with start, end, and content
32+
* @returns {Array} Array of block objects with startNode, endNode, and content
3333
*/
3434
export function parseDirectiveBlocks(tree) {
3535
const blocks = []
@@ -48,7 +48,7 @@ export function parseDirectiveBlocks(tree) {
4848
if (beginMatch) {
4949
if (currentBlock) {
5050
throw new Error(
51-
`Nested BEGIN blocks not allowed. Found BEGIN at line ${lineNumber}, previous BEGIN at line ${currentBlock.start}`,
51+
`Nested BEGIN blocks not allowed. Found BEGIN at line ${lineNumber}, previous BEGIN at line ${currentBlock.startLine}`,
5252
)
5353
}
5454

@@ -58,9 +58,11 @@ export function parseDirectiveBlocks(tree) {
5858
}
5959

6060
currentBlock = {
61-
start: lineNumber,
61+
startNode: node,
62+
startLine: lineNumber,
6263
content: blockContent.trim(),
63-
end: null,
64+
endNode: null,
65+
endLine: null,
6466
}
6567
return
6668
}
@@ -81,13 +83,14 @@ export function parseDirectiveBlocks(tree) {
8183

8284
if (endContent.trim() !== currentBlock.content) {
8385
throw new Error(
84-
`Mismatched block names: BEGIN="${currentBlock.content}" at line ${currentBlock.start}, ` +
86+
`Mismatched block names: BEGIN="${currentBlock.content}" at line ${currentBlock.startLine}, ` +
8587
`END="${endContent.trim()}" at line ${lineNumber}`,
8688
)
8789
}
8890

8991
// Complete the block
90-
currentBlock.end = lineNumber
92+
currentBlock.endNode = node
93+
currentBlock.endLine = lineNumber
9194
blocks.push(currentBlock)
9295
currentBlock = null
9396
}
@@ -96,47 +99,56 @@ export function parseDirectiveBlocks(tree) {
9699
// Check for unclosed blocks
97100
if (currentBlock) {
98101
throw new Error(
99-
`Unclosed BEGIN block: "${currentBlock.content}" opened at line ${currentBlock.start}`,
102+
`Unclosed BEGIN block: "${currentBlock.content}" opened at line ${currentBlock.startLine}`,
100103
)
101104
}
102105

103106
return blocks
104107
}
105108

106109
/**
107-
* Remove nodes from AST within specified line range
110+
* Remove nodes from AST within specified block using node references
108111
*
109-
* This handles nodes from included partials which may not have position data
110-
* matching the parent file's line numbers. We track when we enter/exit the
111-
* removal range based on the BEGIN/END comments.
112+
* This uses actual node object references to identify the range, avoiding
113+
* issues with overlapping line numbers from multiple partials.
112114
*
113115
* @param {Object} tree Remark AST
114-
* @param {number} startLine Start line (inclusive)
115-
* @param {number} endLine End line (inclusive)
116+
* @param {Object} block Block object with startNode and endNode references
116117
*/
117-
export function removeNodesInRange(tree, startLine, endLine) {
118+
export function removeNodesInRange(tree, block) {
119+
const { startNode, endNode } = block
120+
118121
function removeFromNodes(nodes, depth = 0, parentInsideRange = false) {
119122
if (!Array.isArray(nodes)) {
120123
return parentInsideRange
121124
}
122125

123126
const indicesToRemove = []
124127
let insideRange = parentInsideRange
125-
let lastEndLine = null // Track the line number where we last found an END comment
126128

127129
for (let i = 0; i < nodes.length; i++) {
128130
const node = nodes[i]
129-
const hasPosition = node.position?.start?.line && node.position?.end?.line
130131

131-
// Recursively check children FIRST to find any nested END comments
132-
// before deciding whether to remove this node
133-
// const hadChildren = !!node.children
132+
// Check if this is the START node
133+
if (node === startNode) {
134+
insideRange = true
135+
indicesToRemove.push(i)
136+
continue
137+
}
138+
139+
// Check if this is the END node
140+
if (node === endNode) {
141+
indicesToRemove.push(i)
142+
insideRange = false
143+
continue
144+
}
145+
146+
// Recursively check children
134147
const wasInsideRange = insideRange
135148
if (node.children) {
136149
insideRange = removeFromNodes(node.children, depth + 1, insideRange)
137150

138-
// After recursion, children have already been removed.
139-
// Determine whether to remove the parent node:
151+
// After recursion, decide whether to remove the parent node
140152
const nodeIsComment = isCommentNode(node)
141153

142154
// If we were inside range and still are, remove the parent (fully in range)
@@ -146,9 +158,7 @@ export function removeNodesInRange(tree, startLine, endLine) {
146158
}
147159

148160
// If range ENDED inside this node's children (END found in children),
149-
// only remove parent if it's now empty (all children were removed).
150-
// Example: list containing [listItem1 (removed), listItem2 with END (removed), listItem3 (kept)]
151-
// → Keep the list since listItem3 remains
161+
// only remove parent if it's now empty (all children were removed)
152162
if (wasInsideRange && !insideRange) {
153163
if (!node.children || node.children.length === 0) {
154164
indicesToRemove.push(i)
@@ -157,70 +167,15 @@ export function removeNodesInRange(tree, startLine, endLine) {
157167
}
158168

159169
// If range STARTED inside this node's children (BEGIN found in children),
160-
// the parent node CONTAINS the range start and should NOT be removed.
161-
// Example: listItem containing [paragraph, BEGIN comment] - keep the listItem, only BEGIN removed
170+
// the parent node CONTAINS the range start and should NOT be removed
162171
if (!wasInsideRange && insideRange) {
163172
continue
164173
}
165174
}
166175

167-
if (hasPosition) {
168-
const nodeStart = node.position.start.line
169-
const nodeEnd = node.position.end.line
170-
171-
// Check if we've moved past the end of the range
172-
// Only check this for comment nodes (jsx/html/code), as content nodes
173-
// from partials can have any line numbers
174-
const nodeIsComment = isCommentNode(node)
175-
if (
176-
insideRange &&
177-
nodeIsComment &&
178-
nodeStart >= startLine &&
179-
nodeStart > endLine
180-
) {
181-
insideRange = false
182-
}
183-
184-
// If we're NOT inside a range, check if this node starts one
185-
if (!insideRange) {
186-
// Special case: if we just found an END comment on this same line (lastEndLine),
187-
// do NOT start a new range. This prevents removing unrelated BEGIN comments
188-
// that happen to be on the same line as the END comment (e.g., line 24 has both
189-
// "<!-- END: TFC:only -->" and "<!-- BEGIN: TFEnterprise:only -->")
190-
if (lastEndLine !== null && nodeEnd === lastEndLine) {
191-
// This is a BEGIN comment on the same line as the last END - skip it
192-
}
193-
// Check if this node marks the start of the range
194-
// Only jsx/html/code nodes can be BEGIN comments
195-
else if (
196-
nodeIsComment &&
197-
nodeStart >= startLine &&
198-
nodeEnd <= endLine
199-
) {
200-
insideRange = true
201-
indicesToRemove.push(i)
202-
}
203-
}
204-
// If we're inside a range, all nodes are from the partial (except the END comment)
205-
else {
206-
// Check if this is the END comment
207-
// END comments are jsx/html/code nodes with position data from the parent file
208-
if (nodeIsComment && nodeStart >= startLine && nodeEnd === endLine) {
209-
indicesToRemove.push(i)
210-
insideRange = false
211-
lastEndLine = nodeEnd // Remember the line where we found the END comment
212-
}
213-
// Otherwise, it's a partial node - remove it
214-
else {
215-
indicesToRemove.push(i)
216-
}
217-
}
218-
} else {
219-
// Node without position (e.g., from included partial)
220-
// Remove it if we're currently inside the range
221-
if (insideRange) {
222-
indicesToRemove.push(i)
223-
}
176+
// If we're inside the range and this node isn't a boundary comment, remove it
177+
if (insideRange) {
178+
indicesToRemove.push(i)
224179
}
225180
}
226181

scripts/prebuild/mdx-transforms/exclude-content/terraform-processor.mjs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { removeNodesInRange } from './ast-utils.mjs'
1010
* Implements cross-product behavior based on your test requirements
1111
*
1212
* @param {string} directive The directive part (e.g., "only")
13-
* @param {Object} block Block information with start, end, content
13+
* @param {Object} block Block information with startNode, endNode, content
1414
* @param {Object} tree Remark AST
1515
* @param {Object} options Processing options
1616
*/
@@ -21,15 +21,15 @@ export function processTFCBlock(directive, block, tree, options) {
2121
if (directive === 'only' || directive.startsWith('only ')) {
2222
// TFC:only should be kept ONLY in terraform-docs-common, removed everywhere else
2323
if (repoSlug !== 'terraform-docs-common') {
24-
removeNodesInRange(tree, block.start, block.end)
24+
removeNodesInRange(tree, block)
2525
}
2626
// else keeping block in terraform-docs-common
2727
return
2828
}
2929

3030
// If we get here, it's an invalid TFC directive
3131
throw new Error(
32-
`Invalid TFC directive: "${directive}" at lines ${block.start}-${block.end}. ` +
32+
`Invalid TFC directive: "${directive}" at lines ${block.startLine}-${block.endLine}. ` +
3333
`Expected format: TFC:only`,
3434
)
3535
}
@@ -39,7 +39,7 @@ export function processTFCBlock(directive, block, tree, options) {
3939
* Implements cross-product behavior based on your test requirements
4040
*
4141
* @param {string} directive The directive part (e.g., "only")
42-
* @param {Object} block Block information with start, end, content
42+
* @param {Object} block Block information with startNode, endNode, content
4343
* @param {Object} tree Remark AST
4444
* @param {Object} options Processing options
4545
*/
@@ -50,14 +50,15 @@ export function processTFEnterpriseBlock(directive, block, tree, options) {
5050
if (directive === 'only' || directive.startsWith('only ')) {
5151
// TFEnterprise:only kept only in terraform-enterprise, removed everywhere else
5252
if (repoSlug !== 'terraform-enterprise') {
53-
removeNodesInRange(tree, block.start, block.end)
53+
removeNodesInRange(tree, block)
5454
}
5555
// else keeping block (in terraform-enterprise)`)
5656
return
5757
}
5858

5959
// If we get here, it's an invalid TFEnterprise directive
6060
throw new Error(
61-
`Invalid TFEnterprise directive: "${directive}" at lines ${block.start}-${block.end}. Expected format: TFEnterprise:only`,
61+
`Invalid TFEnterprise directive: "${directive}" at lines ${block.startLine}-${block.endLine}. ` +
62+
`Expected format: TFEnterprise:only`,
6263
)
6364
}

scripts/prebuild/mdx-transforms/exclude-content/vault-processor.mjs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { removeNodesInRange } from './ast-utils.mjs'
1111
* Only processes Vault directives in vault files - ignores them elsewhere
1212
*
1313
* @param {string} directive The directive part (e.g., ">=v1.21.x")
14-
* @param {Object} block Block information with start, end, content
14+
* @param {Object} block Block information with startNode, endNode, content
1515
* @param {Object} tree Remark AST
1616
* @param {Object} options Processing options
1717
*/
@@ -32,7 +32,7 @@ export function processVaultBlock(directive, block, tree, options) {
3232

3333
// If we get here, it's an invalid Vault directive
3434
throw new Error(
35-
`Invalid Vault directive: "${directive}" at lines ${block.start}-${block.end}. ` +
35+
`Invalid Vault directive: "${directive}" at lines ${block.startLine}-${block.endLine}. ` +
3636
`Expected format: Vault:>=vX.Y.x`,
3737
)
3838
}
@@ -46,7 +46,7 @@ function processVaultVersionDirective(versionMatch, block, tree, options) {
4646

4747
if (!version) {
4848
throw new Error(
49-
`Version directive requires version option at lines ${block.start}-${block.end}`,
49+
`Version directive requires version option at lines ${block.startLine}-${block.endLine}`,
5050
)
5151
}
5252

@@ -59,11 +59,11 @@ function processVaultVersionDirective(versionMatch, block, tree, options) {
5959

6060
// If version comparison fails, remove the content
6161
if (!shouldKeepContent) {
62-
removeNodesInRange(tree, block.start, block.end)
62+
removeNodesInRange(tree, block)
6363
}
6464
} catch (error) {
6565
throw new Error(
66-
`Version comparison failed for "${block.content}" at lines ${block.start}-${block.end}: ${error.message}`,
66+
`Version comparison failed for "${block.content}" at lines ${block.startLine}-${block.endLine}: ${error.message}`,
6767
)
6868
}
6969
}

0 commit comments

Comments
 (0)