-
Notifications
You must be signed in to change notification settings - Fork 79
refactor: use getLocFromIndex
in no-space-in-emphasis
#553
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
mdjermanovic
merged 10 commits into
main
from
refactor-use-getlocfromindex-in-no-space-in-emphasis
Oct 7, 2025
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
6ee8076
refactor: use `getLocFromIndex` in `no-space-in-emphasis`
lumirlumir b942530
wip: simplify
lumirlumir c45cefe
wip: simplify
lumirlumir ea7c2f0
wip: simplify
lumirlumir 27e81ac
wip: simplify
lumirlumir 2396ced
wip: simplify
lumirlumir 5d3b62e
wip
lumirlumir f74fcc8
wip: simplify
lumirlumir 24b0b7e
wip
lumirlumir f70c88a
wip
lumirlumir File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -3,17 +3,12 @@ | |||||
* @author Pixel998 | ||||||
*/ | ||||||
|
||||||
//----------------------------------------------------------------------------- | ||||||
// Imports | ||||||
//----------------------------------------------------------------------------- | ||||||
|
||||||
import { findOffsets } from "../util.js"; | ||||||
|
||||||
//----------------------------------------------------------------------------- | ||||||
// Type Definitions | ||||||
//----------------------------------------------------------------------------- | ||||||
|
||||||
/** | ||||||
* @import { SourceRange } from "@eslint/core"; | ||||||
* @import { Heading, Paragraph, TableCell, Text } from "mdast"; | ||||||
* @import { MarkdownRuleDefinition } from "../types.js"; | ||||||
* @typedef {"spaceInEmphasis"} NoSpaceInEmphasisMessageIds | ||||||
|
@@ -34,32 +29,8 @@ const whitespacePattern = /[ \t]/u; | |||||
*/ | ||||||
function createMarkerPattern(checkStrikethrough) { | ||||||
return checkStrikethrough | ||||||
? /(?<=(?<!\\)(?:\\{2})*)(?<marker>\*\*\*|\*\*|\*|___|__|_|~~|~)/gu | ||||||
: /(?<=(?<!\\)(?:\\{2})*)(?<marker>\*\*\*|\*\*|\*|___|__|_)/gu; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Finds all emphasis markers in the text. | ||||||
* @param {string} text The text to search. | ||||||
* @param {RegExp} pattern The marker pattern to use. | ||||||
* @typedef {{marker: string, startIndex: number, endIndex: number}} EmphasisMarker | ||||||
* @returns {Array<EmphasisMarker>} Array of emphasis markers. | ||||||
*/ | ||||||
function findEmphasisMarkers(text, pattern) { | ||||||
/** @type {Array<EmphasisMarker>} */ | ||||||
const markers = []; | ||||||
/** @type {RegExpExecArray | null} */ | ||||||
let match; | ||||||
|
||||||
while ((match = pattern.exec(text)) !== null) { | ||||||
markers.push({ | ||||||
marker: match.groups.marker, | ||||||
startIndex: match.index, | ||||||
endIndex: match.index + match.groups.marker.length, | ||||||
}); | ||||||
} | ||||||
|
||||||
return markers; | ||||||
? /(?<=(?<!\\)(?:\\{2})*)(?:\*\*\*|\*\*|\*|___|__|_|~~|~)/gu | ||||||
: /(?<=(?<!\\)(?:\\{2})*)(?:\*\*\*|\*\*|\*|___|__|_)/gu; | ||||||
} | ||||||
|
||||||
//----------------------------------------------------------------------------- | ||||||
|
@@ -107,182 +78,97 @@ export default { | |||||
const [{ checkStrikethrough }] = context.options; | ||||||
const markerPattern = createMarkerPattern(checkStrikethrough); | ||||||
|
||||||
/** @type {string[]} */ | ||||||
let buffer; | ||||||
|
||||||
/** | ||||||
* Reports a surrounding-space violation if present. | ||||||
* @param {Object} params Options for the report arguments. | ||||||
* @param {string} params.originalText The original text of the node. | ||||||
* @param {number} params.checkIndex Character index to test for whitespace. | ||||||
* @param {number} params.highlightStartIndex Start index for highlighting. | ||||||
* @param {number} params.highlightEndIndex End index for highlighting. | ||||||
* @param {number} params.removeIndex Absolute index of the space to remove. | ||||||
* @param {number} params.nodeStartLine The starting line number for the node. | ||||||
* @param {number} params.nodeStartColumn The starting column number for the node. | ||||||
* @param {number} checkIndex Character index to test for whitespace. | ||||||
* @param {number} highlightStartIndex Start index for highlighting. | ||||||
* @param {number} highlightEndIndex End index for highlighting. | ||||||
* @returns {void} | ||||||
*/ | ||||||
function reportWhitespace({ | ||||||
originalText, | ||||||
function reportWhitespace( | ||||||
checkIndex, | ||||||
highlightStartIndex, | ||||||
highlightEndIndex, | ||||||
removeIndex, | ||||||
nodeStartLine, | ||||||
nodeStartColumn, | ||||||
}) { | ||||||
if (whitespacePattern.test(originalText[checkIndex])) { | ||||||
const { | ||||||
lineOffset: startLineOffset, | ||||||
columnOffset: startColumnOffset, | ||||||
} = findOffsets(originalText, highlightStartIndex); | ||||||
const { | ||||||
lineOffset: endLineOffset, | ||||||
columnOffset: endColumnOffset, | ||||||
} = findOffsets(originalText, highlightEndIndex); | ||||||
|
||||||
) { | ||||||
if (whitespacePattern.test(sourceCode.text[checkIndex])) { | ||||||
context.report({ | ||||||
loc: { | ||||||
start: { | ||||||
line: nodeStartLine + startLineOffset, | ||||||
column: nodeStartColumn + startColumnOffset, | ||||||
}, | ||||||
end: { | ||||||
line: nodeStartLine + endLineOffset, | ||||||
column: nodeStartColumn + endColumnOffset, | ||||||
}, | ||||||
start: sourceCode.getLocFromIndex(highlightStartIndex), | ||||||
end: sourceCode.getLocFromIndex(highlightEndIndex), | ||||||
}, | ||||||
messageId: "spaceInEmphasis", | ||||||
fix(fixer) { | ||||||
return fixer.removeRange([ | ||||||
removeIndex, | ||||||
removeIndex + 1, | ||||||
]); | ||||||
return fixer.removeRange([checkIndex, checkIndex + 1]); | ||||||
}, | ||||||
}); | ||||||
} | ||||||
} | ||||||
|
||||||
/** | ||||||
* Checks a given node for emphasis markers with surrounding spaces. | ||||||
* @param {Heading|Paragraph|TableCell} node The node to check. | ||||||
* @param {string} maskedText The masked text preserving only direct text content. | ||||||
* @returns {void} | ||||||
*/ | ||||||
function checkEmphasis(node, maskedText) { | ||||||
const originalText = sourceCode.getText(node); | ||||||
const markers = findEmphasisMarkers(maskedText, markerPattern); | ||||||
const nodeStartLine = node.position.start.line; | ||||||
const nodeStartColumn = node.position.start.column; | ||||||
const nodeStartOffset = node.position.start.offset; | ||||||
|
||||||
const markerGroups = new Map(); | ||||||
for (const marker of markers) { | ||||||
if (!markerGroups.has(marker.marker)) { | ||||||
markerGroups.set(marker.marker, []); | ||||||
} | ||||||
markerGroups.get(marker.marker).push(marker); | ||||||
} | ||||||
|
||||||
for (const group of markerGroups.values()) { | ||||||
for (let i = 0; i < group.length - 1; i += 2) { | ||||||
const startMarker = group[i]; | ||||||
reportWhitespace({ | ||||||
originalText, | ||||||
checkIndex: startMarker.endIndex, | ||||||
highlightStartIndex: startMarker.startIndex, | ||||||
highlightEndIndex: startMarker.endIndex + 2, | ||||||
removeIndex: nodeStartOffset + startMarker.endIndex, | ||||||
nodeStartLine, | ||||||
nodeStartColumn, | ||||||
}); | ||||||
|
||||||
const endMarker = group[i + 1]; | ||||||
reportWhitespace({ | ||||||
originalText, | ||||||
checkIndex: endMarker.startIndex - 1, | ||||||
highlightStartIndex: endMarker.startIndex - 2, | ||||||
highlightEndIndex: endMarker.endIndex, | ||||||
removeIndex: nodeStartOffset + endMarker.startIndex - 1, | ||||||
nodeStartLine, | ||||||
nodeStartColumn, | ||||||
}); | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
/** | ||||||
* Manager for building a masked character array for a node's direct text content. | ||||||
* @typedef {{ buffer: string[], startOffset: number }} BufferState | ||||||
*/ | ||||||
const bufferManager = { | ||||||
/** @type {BufferState | null} */ | ||||||
state: null, | ||||||
|
||||||
/** | ||||||
* Initialize state with a whitespace-masked character buffer for the node. | ||||||
* @param {Heading|Paragraph|TableCell} node Heading, Paragraph, or TableCell node to enter. | ||||||
* @returns {void} | ||||||
*/ | ||||||
enter(node) { | ||||||
return { | ||||||
"heading, paragraph, tableCell"( | ||||||
/** @type {Heading | Paragraph | TableCell} */ node, | ||||||
) { | ||||||
const [startOffset, endOffset] = sourceCode.getRange(node); | ||||||
this.state = { | ||||||
buffer: new Array(endOffset - startOffset).fill(" "), | ||||||
startOffset, | ||||||
}; | ||||||
}, | ||||||
|
||||||
/** | ||||||
* Add the content of a Text node into the current buffer at the correct offsets. | ||||||
* @param {Text} node Text node whose characters will be copied into the buffer. | ||||||
* @returns {void} | ||||||
*/ | ||||||
addText(node) { | ||||||
const start = | ||||||
node.position.start.offset - this.state.startOffset; | ||||||
const text = sourceCode.getText(node); | ||||||
for (let i = 0; i < text.length; i++) { | ||||||
this.state.buffer[start + i] = text[i]; | ||||||
} | ||||||
// Initialize `buffer` with a whitespace-masked character array. | ||||||
buffer = new Array(endOffset - startOffset).fill(" "); | ||||||
}, | ||||||
|
||||||
/** | ||||||
* Join the character buffer into a masked string, run checks, then clear state. | ||||||
* @param {Heading|Paragraph|TableCell} node Heading, Paragraph, or TableCell node to exit. | ||||||
* @returns {void} | ||||||
*/ | ||||||
exit(node) { | ||||||
checkEmphasis(node, this.state.buffer.join("")); | ||||||
this.state = null; | ||||||
}, | ||||||
}; | ||||||
":matches(heading, paragraph, tableCell) > text"( | ||||||
/** @type {Text} */ node, | ||||||
) { | ||||||
const [startOffset, endOffset] = sourceCode.getRange(node); | ||||||
const parentNodeStartOffset = // Parent node can be `Heading`, `Paragraph`, or `TableCell`. | ||||||
sourceCode.getParent(node).position.start.offset; | ||||||
|
||||||
return { | ||||||
heading(node) { | ||||||
bufferManager.enter(node); | ||||||
}, | ||||||
"heading > text"(node) { | ||||||
bufferManager.addText(node); | ||||||
}, | ||||||
"heading:exit"(node) { | ||||||
bufferManager.exit(node); | ||||||
// Add the content of a `Text` node into the current buffer at the correct offsets. | ||||||
for (let i = startOffset; i < endOffset; i++) { | ||||||
buffer[i - parentNodeStartOffset] = sourceCode.text[i]; | ||||||
} | ||||||
}, | ||||||
|
||||||
paragraph(node) { | ||||||
bufferManager.enter(node); | ||||||
}, | ||||||
"paragraph > text"(node) { | ||||||
bufferManager.addText(node); | ||||||
}, | ||||||
"paragraph:exit"(node) { | ||||||
bufferManager.exit(node); | ||||||
}, | ||||||
":matches(heading, paragraph, tableCell):exit"( | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
The |
||||||
/** @type {Heading | Paragraph | TableCell} */ node, | ||||||
) { | ||||||
const maskedText = buffer.join(""); | ||||||
/** @type {Map<string, SourceRange[]>} */ | ||||||
const markerGroups = new Map(); | ||||||
|
||||||
/** @type {RegExpExecArray | null} */ | ||||||
let match; | ||||||
|
||||||
while ((match = markerPattern.exec(maskedText)) !== null) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could use String#matchAll instead of using
Suggested change
|
||||||
const marker = match[0]; | ||||||
const startOffset = // Adjust `markerPattern` match index to the full source code. | ||||||
match.index + node.position.start.offset; | ||||||
const endOffset = startOffset + marker.length; | ||||||
|
||||||
if (!markerGroups.has(marker)) { | ||||||
markerGroups.set(marker, []); | ||||||
} | ||||||
markerGroups.get(marker).push([startOffset, endOffset]); | ||||||
} | ||||||
|
||||||
tableCell(node) { | ||||||
bufferManager.enter(node); | ||||||
}, | ||||||
"tableCell > text"(node) { | ||||||
bufferManager.addText(node); | ||||||
}, | ||||||
"tableCell:exit"(node) { | ||||||
bufferManager.exit(node); | ||||||
for (const group of markerGroups.values()) { | ||||||
for (let i = 0; i < group.length - 1; i += 2) { | ||||||
const startMarker = group[i]; | ||||||
reportWhitespace( | ||||||
startMarker[1], | ||||||
startMarker[0], | ||||||
startMarker[1] + 2, | ||||||
); | ||||||
|
||||||
const endMarker = group[i + 1]; | ||||||
reportWhitespace( | ||||||
endMarker[0] - 1, | ||||||
endMarker[0] - 2, | ||||||
endMarker[1], | ||||||
); | ||||||
} | ||||||
} | ||||||
}, | ||||||
}; | ||||||
}, | ||||||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also remove the function as part of this refactoring as no regular expression is dynamically created?