Skip to content

Commit

Permalink
only show no-undef errors for templates in gts files
Browse files Browse the repository at this point in the history
  • Loading branch information
patricklx committed May 11, 2023
1 parent 208e6b6 commit eee5cc2
Show file tree
Hide file tree
Showing 3 changed files with 158 additions and 2 deletions.
59 changes: 58 additions & 1 deletion lib/preprocessors/glimmer.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const {
preprocessEmbeddedTemplates,
} = require('ember-template-imports/lib/preprocess-embedded-templates');
const util = require('ember-template-imports/src/util');
const DocumentLines = require('../utils/document');

const TRANSFORM_CACHE = new Map();
const TEXT_CACHE = new Map();
Expand All @@ -27,6 +28,12 @@ const closingTemplateTagRegex = /`, {.*}\)]/;
// the capture group would contain `on,noop`
const getScopeTokenRegex = /scope: \(\) => \({(.*)}\) }\)/;

// eslint rules that check for undefined identifiers
// this are the only rules we want to check within a template
// Because otherwise eslint would show errors unrelated to the original code and
// apply auto fixes like spacing, comma etc. into the original template
const RULES_THAT_CHECK_FOR_UNDEFINED_IDENTIFIERS = new Set(['no-undef']);

/**
* This function is responsible for running the embedded templates transform
* from ember-template-imports.
Expand Down Expand Up @@ -86,6 +93,7 @@ function gjs(text, filename) {
//
// We need to disable all of eslint around this "scope" return
// value and allow-list the rules that check for undefined identifiers
// this also ensures that no errors about the generated output appear.
const transformed = preprocessed.output;

TRANSFORM_CACHE.set(filename, transformed);
Expand Down Expand Up @@ -120,9 +128,40 @@ function mapRange(messages, filename) {
return flattened;
}

const transformedDoc = new DocumentLines(transformed);

const lines = transformed.split('\n');
const originalLines = original.split('\n');

// get template ranges to remove eslint messages that are inside the templates
const templateRanges = [];
let isInsideTemplate = false;
for (const [index, line] of lines.entries()) {
const lineHasOpeningTag = openingTemplateTagRegex.test(line);
const lineHasClosingTag = closingTemplateTagRegex.test(line);

if (lineHasOpeningTag) {
isInsideTemplate = true;
templateRanges.push({
start: transformedDoc.positionToOffset({
line: index,
character: line.search(openingTemplateTagRegex),
}),
end: -1,
});
}

if (lineHasClosingTag && isInsideTemplate) {
isInsideTemplate = false;
const match = line.match(closingTemplateTagRegex);
const current = templateRanges.slice(-1)[0];
current.end = transformedDoc.positionToOffset({
line: index,
character: match.index + match[0].length,
});
}
}

// this function assumes the original output and transformed output
// are identical, minus the changes to transform `<template>` into the
// placeholder `[__GLIMMER_TEMPLATE()]
Expand All @@ -135,7 +174,25 @@ function mapRange(messages, filename) {
);
}

return flattened.map((message) => {
const filtered = flattened.filter((it) => {
const start = transformedDoc.positionToOffset({
line: it.line - 1,
character: it.column - 1,
});
const end = transformedDoc.positionToOffset({
line: it.endLine - 1,
character: it.endColumn - 1,
});
return (
RULES_THAT_CHECK_FOR_UNDEFINED_IDENTIFIERS.has(it.ruleId) ||
// include if message wraps template
templateRanges.some((tpl) => start === tpl.start && end === tpl.end) ||
// exclude if message is within template (also when it ends on next line, column 1, so +1 char, e.g semi rule)
!templateRanges.some((tpl) => start >= tpl.start && end <= tpl.end + 1)
);
});

return filtered.map((message) => {
// 1. handle eslint diagnostics on single lines.
if (message.line === message.endLine) {
const originalLine = originalLines[message.line - 1];
Expand Down
97 changes: 97 additions & 0 deletions lib/utils/document.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/**
* @typedef {{ line: number; character: number }} Position
*/

class DocumentLines {
/**
* @param {string} contents
*/
constructor(contents) {
this.lineStarts = computeLineStarts(contents);
}

/**
* @param {Position} position
* @return {number}
*/
positionToOffset(position) {
const { line, character } = position;
return this.lineStarts[line] + character;
}

/**
*
* @param {number} position
* @return {Position}
*/
offsetToPosition(position) {
const lineStarts = this.lineStarts;
let line = 0;
while (line + 1 < lineStarts.length && lineStarts[line + 1] <= position) {
line++;
}
const character = position - lineStarts[line];
return { line, character };
}
}

/**
* @returns {number[]}
* @param {string} text
*/
function computeLineStarts(text) {
const result = [];
let pos = 0;
let lineStart = 0;
while (pos < text.length) {
const ch = text.codePointAt(pos);
pos++;
switch (ch) {
case 13 /* carriageReturn */: {
if (text.codePointAt(pos) === 10 /* lineFeed */) {
pos++;
}
}
// falls through
case 10 /* lineFeed */: {
result.push(lineStart);
lineStart = pos;
break;
}
default: {
if (ch > 127 /* maxAsciiCharacter */ && isLineBreak(ch)) {
result.push(lineStart);
lineStart = pos;
}
break;
}
}
}
result.push(lineStart);
return result;
}

/**
* @param {number} ch
* @return {boolean}
*/
function isLineBreak(ch) {
// ES5 7.3:
// The ECMAScript line terminator characters are listed in Table 3.
// Table 3: Line Terminator Characters
// Code Unit Value Name Formal Name
// \u000A Line Feed <LF>
// \u000D Carriage Return <CR>
// \u2028 Line separator <LS>
// \u2029 Paragraph separator <PS>
// Only the characters in Table 3 are treated as line terminators. Other new line or line
// breaking characters are treated as white space but not as line terminators.
return (
ch === 10 /* lineFeed */ ||
ch === 13 /* carriageReturn */ ||
ch === 8232 /* lineSeparator */ ||
ch === 8233 /* paragraphSeparator */
);
}

module.exports = DocumentLines;
4 changes: 3 additions & 1 deletion tests/lib/rules-preprocessor/gjs-gts-processor-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ function initESLint(options) {
plugins: ['ember'],
extends: ['plugin:ember/recommended'],
rules: {
semi: ['error', 'always'],
'object-curly-spacing': ['error', 'always'],
'lines-between-class-members': 'error',
'no-undef': 'error',
'no-unused-vars': 'error',
Expand Down Expand Up @@ -388,7 +390,7 @@ describe('multiple tokens in same file', () => {
it('handles duplicate template tokens', async () => {
const eslint = initESLint();
const code = `
// comment Bad
// comment Bad
const tmpl = <template><Bad /></template>
`;
Expand Down

0 comments on commit eee5cc2

Please sign in to comment.