From 40e019161a29856b8bb7c4df7217ca239f087482 Mon Sep 17 00:00:00 2001 From: Grace Park Date: Thu, 30 May 2024 08:52:14 -0700 Subject: [PATCH] Fix linter crash with multiple badly formatted liquid data tags (#50849) --- .../contributing/content-linter-rules.md | 2 +- .../lib/linting-rules/liquid-data-tags.js | 51 ++++++++++++++----- .../scripts/pretty-print-results.js | 49 ++++++++++++++++-- 3 files changed, 85 insertions(+), 17 deletions(-) diff --git a/data/reusables/contributing/content-linter-rules.md b/data/reusables/contributing/content-linter-rules.md index d0dabc9893..f6fce7f6e1 100644 --- a/data/reusables/contributing/content-linter-rules.md +++ b/data/reusables/contributing/content-linter-rules.md @@ -48,7 +48,7 @@ | GHD013 | github-owned-action-references | GitHub-owned action references should not be hardcoded | error | feature, actions | | GHD016 | liquid-quoted-conditional-arg | Liquid conditional tags should not quote the conditional argument | error | liquid, format | | GHD014 | liquid-data-references-defined | Liquid data or indented data references were found in content that have no value or do not exist in the data directory | error | liquid | -| GHD015 | liquid-data-tag-format | Liquid data or indented data references tags must have the correct number of arguments and spacing | error | liquid, format | +| GHD015 | liquid-data-tag-format | Liquid data or indented data references tags must be correctly formatted and have the correct number of arguments and spacing | error | liquid, format | | GHD010 | frontmatter-hidden-docs | Articles with frontmatter property `hidden` can only be located in specific products | error | frontmatter, feature, early-access | | GHD009 | frontmatter-early-access-references | Files that are not early access should not have frontmatter that references early-access | error | frontmatter, feature, early-access | | GHD011 | frontmatter-video-transcripts | Video transcript must be configured correctly | error | frontmatter, feature, video-transcripts | diff --git a/src/content-linter/lib/linting-rules/liquid-data-tags.js b/src/content-linter/lib/linting-rules/liquid-data-tags.js index ca5491f8f4..ff85cbd819 100644 --- a/src/content-linter/lib/linting-rules/liquid-data-tags.js +++ b/src/content-linter/lib/linting-rules/liquid-data-tags.js @@ -2,7 +2,12 @@ import { TokenKind } from 'liquidjs' import { addError } from 'markdownlint-rule-helpers' import { getDataByLanguage } from '#src/data-directory/lib/get-data.js' -import { getLiquidTokens, getPositionData } from '../helpers/liquid-utils.js' +import { + getLiquidTokens, + getPositionData, + OUTPUT_OPEN, + OUTPUT_CLOSE, +} from '../helpers/liquid-utils.js' /* Checks for instances where a Liquid data or indented_data_reference @@ -13,6 +18,7 @@ export const liquidDataReferencesDefined = { description: 'Liquid data or indented data references were found in content that have no value or do not exist in the data directory', tags: ['liquid'], + parser: 'markdownit', function: (params, onError) => { const content = params.lines.join('\n') const tokens = getLiquidTokens(content) @@ -46,9 +52,10 @@ export const liquidDataReferencesDefined = { export const liquidDataTagFormat = { names: ['GHD015', 'liquid-data-tag-format'], description: - 'Liquid data or indented data references tags must have the correct number of arguments and spacing', + 'Liquid data or indented data references tags must be correctly formatted and have the correct number of arguments and spacing', tags: ['liquid', 'format'], function: (params, onError) => { + const CHECK_LIQUID_TAGS = [OUTPUT_OPEN, OUTPUT_CLOSE, '{', '}'] const content = params.lines.join('\n') const tokenTags = getLiquidTokens(content).filter((token) => token.kind === TokenKind.Tag) const dataTags = tokenTags.filter((token) => token.name === 'data') @@ -61,17 +68,37 @@ export const liquidDataTagFormat = { // split() returns [''], so we need to check for that case. if (args.length === 1 && token.args !== '') continue - const lines = params.lines - const { lineNumber, column, length } = getPositionData(token, lines) + // When we filter out the data tokens from getLiquidTokens, we are left with the data content itself + // without the liquid opening/closing tags. If we see that it is in the args of the token, we can + // assume that the data tag is not formatted correctly. + // This is not necessary as the liquid tests will later catch badly formatted liquid, but badly + // formatted data tags prevents getting the correct position data for the test below. + const containsBadLiquidDataTags = CHECK_LIQUID_TAGS.some((tag) => token.args.includes(tag)) - addError( - onError, - lineNumber, - `The Liquid data tag {% ${token.content} %} can only have one argument but includes ${args.length}.`, - token.content, - [column, length], - null, // No fix available - ) + if (containsBadLiquidDataTags) { + const lines = params.lines + const { lineNumber } = getPositionData(token, lines) + addError( + onError, + lineNumber, + `This content contains incorrectly formatted Liquid data tag(s): ${token.content}.`, + token.content, + '', + null, // No fix available + ) + } else { + const lines = params.lines + const { lineNumber, column, length } = getPositionData(token, lines) + + addError( + onError, + lineNumber, + `The Liquid data tag {% ${token.content} %} can only have one argument but includes ${args.length}.`, + token.content, + [column, length], + null, // No fix available + ) + } } for (const token of indentedDataTags) { diff --git a/src/content-linter/scripts/pretty-print-results.js b/src/content-linter/scripts/pretty-print-results.js index 9f1098e433..77f586b7b0 100644 --- a/src/content-linter/scripts/pretty-print-results.js +++ b/src/content-linter/scripts/pretty-print-results.js @@ -69,13 +69,17 @@ export function prettyPrintResults(results, { fixed = false } = {}) { if (result.fixable && !fixed) { console.log(`${PREFIX_PADDING}${chalk.green('Fixable!')}`) } + const ruleNames = result.ruleNames.map((name) => chalk.bold(name)).join(', ') console.log( label('Rule'), - result.ruleNames.map((name) => chalk.bold(name)).join(', '), - chalk.dim(result.ruleDescription), + ruleNames, + chalk.dim(indentWrappedString(result.ruleDescription, ruleNames.length)), ) if (!distinctDetails) { - console.log(label('Detail'), result.errorDetail) + console.log( + label('Detail'), + `${indentWrappedString(result.errorDetail?.replace(/\n/g, ' ').trim(), PREFIX_PADDING.length * 8)}`, + ) } } let position = chalk.yellow(result.lineNumber) @@ -83,7 +87,13 @@ export function prettyPrintResults(results, { fixed = false } = {}) { position += ` (col ${chalk.yellow(result.columnNumber)})` } if (distinctDetails) { - console.log(label('Detail'), result.errorDetail) + console.log( + label('Detail'), + indentWrappedString( + result.errorDetail?.replace(/\n/g, ' ').trim(), + PREFIX_PADDING.length * 8, + ), + ) } console.log( label('Line'), @@ -120,3 +130,34 @@ function chalkFunColors(text) { }) .join('') } + +function indentWrappedString(str, startingIndent) { + const NEW_LINE_PADDING = ' '.repeat(16) + const width = process.stdout.columns || 80 // Use terminal width, default to 80 if not available + let indentedString = '' + let currentLine = '' + let isFirstLine = true + + for (const word of str.split(' ')) { + const effectiveWidth = isFirstLine ? width - startingIndent : width - NEW_LINE_PADDING.length + + if ((currentLine + word).length > effectiveWidth) { + if (isFirstLine) { + indentedString += currentLine.trim() + '\n' + isFirstLine = false + } else { + indentedString += NEW_LINE_PADDING + currentLine.trim() + '\n' + } + currentLine = word + ' ' + } else { + currentLine += word + ' ' + } + } + if (isFirstLine) { + indentedString += currentLine.trim() + } else { + indentedString += NEW_LINE_PADDING + currentLine.trim() + } + + return indentedString +}