Skip to content
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

Improve astro check #3906

Merged
merged 8 commits into from
Jul 18, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/astro/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
},
"dependencies": {
"@astrojs/compiler": "^0.19.0",
"@astrojs/language-server": "^0.13.4",
"@astrojs/language-server": "^0.20.0",
"@astrojs/markdown-remark": "^0.11.7",
"@astrojs/prism": "0.6.0",
"@astrojs/telemetry": "^0.4.0",
Expand Down
117 changes: 0 additions & 117 deletions packages/astro/src/cli/check.ts

This file was deleted.

96 changes: 96 additions & 0 deletions packages/astro/src/cli/check/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/* eslint-disable no-console */
import { AstroCheck, DiagnosticSeverity } from '@astrojs/language-server';
import type { AstroConfig } from '../../@types/astro';

import glob from 'fast-glob';
import * as fs from 'fs';
import { bold, dim, red, yellow } from 'kleur/colors';
import ora from 'ora';
import { fileURLToPath, pathToFileURL } from 'url';
import { printDiagnostic } from './print.js';

interface Result {
errors: number;
// The language server cannot actually return any warnings at the moment, but we'll keep this here for future use
warnings: number;
hints: number;
}

export async function check(astroConfig: AstroConfig) {
console.log(bold('astro check'));

const root = astroConfig.root;

const spinner = ora(` Getting diagnostics for Astro files in ${fileURLToPath(root)}…`).start();

let checker = new AstroCheck(root.toString());
const filesCount = await openAllDocuments(root, [], checker);

let diagnostics = await checker.getDiagnostics();

spinner.succeed();

let result: Result = {
errors: 0,
warnings: 0,
hints: 0,
};

diagnostics.forEach((diag) => {
diag.diagnostics.forEach((d) => {
console.log(printDiagnostic(diag.filePath, diag.text, d));

switch (d.severity) {
case DiagnosticSeverity.Error: {
result.errors++;
break;
}
case DiagnosticSeverity.Warning: {
result.warnings++;
break;
}
case DiagnosticSeverity.Hint: {
result.hints++;
break;
}
}
});
});

console.log(
[
bold(`Result (${filesCount} file${filesCount === 1 ? '' : 's'}): `),
bold(red(`${result.errors} ${result.errors === 1 ? 'error' : 'errors'}`)),
bold(yellow(`${result.warnings} ${result.warnings === 1 ? 'warning' : 'warnings'}`)),
dim(`${result.hints} ${result.hints === 1 ? 'hint' : 'hints'}\n`),
].join(`\n${dim('-')} `)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the summary section

);

const exitCode = result.errors ? 1 : 0;
return exitCode;
}

/**
* Open all Astro files in the given directory and return the number of files found.
*/
async function openAllDocuments(
workspaceUri: URL,
filePathsToIgnore: string[],
checker: AstroCheck
): Promise<number> {
const files = await glob('**/*.astro', {
cwd: fileURLToPath(workspaceUri),
ignore: ['node_modules/**'].concat(filePathsToIgnore.map((ignore) => `${ignore}/**`)),
absolute: true,
});

for (const file of files) {
const text = fs.readFileSync(file, 'utf-8');
checker.upsertDocument({
uri: pathToFileURL(file).toString(),
text,
});
}

return files.length;
}
118 changes: 118 additions & 0 deletions packages/astro/src/cli/check/print.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import { Diagnostic, DiagnosticSeverity, offsetAt } from '@astrojs/language-server';
import {
bgRed,
bgWhite,
bgYellow,
black,
bold,
cyan,
gray,
red,
white,
yellow
} from 'kleur/colors';
import stringWidth from 'string-width';

export function printDiagnostic(filePath: string, text: string, diag: Diagnostic): string {
let result = [];

// Lines and characters are 0-indexed, so we need to add 1 to the offset to get the actual line and character
const realStartLine = diag.range.start.line + 1;
const realStartCharacter = diag.range.start.character + 1;

// IDE friendly path that user can CTRL+Click to open the file at a specific line / character
const IDEFilePath = `${bold(cyan(filePath))}:${bold(yellow(realStartLine))}:${bold(
yellow(realStartCharacter)
)}`;
result.push(
`${IDEFilePath} ${bold(getColorForSeverity(diag, getStringForSeverity(diag)))}: ${diag.message}`
);

// Optionally add the before the error to add context if not empty
const previousLine = getLine(diag.range.start.line - 1, text);
if (previousLine) {
result.push(`${getPrintableLineNumber(realStartLine - 1)} ${gray(previousLine)}`);
}

// Add the line with the error
const str = getLine(diag.range.start.line, text);
const lineNumStr = realStartLine.toString().padStart(2, '0');
const lineNumLen = lineNumStr.length;
result.push(`${getBackgroundForSeverity(diag, lineNumStr)} ${str}`);

// Adds tildes under the specific range where the diagnostic is
const tildes = generateString('~', diag.range.end.character - diag.range.start.character);

// NOTE: This is not perfect, if the line include any characters that is made of multiple characters, for example
// regionals flags, but the terminal can't display it, then the number of spaces will be wrong. Not sure how to fix.
const beforeChars = stringWidth(str.substring(0, diag.range.start.character));
const spaces = generateString(' ', beforeChars + lineNumLen - 1);
result.push(` ${spaces}${bold(getColorForSeverity(diag, tildes))}`);

const nextLine = getLine(diag.range.start.line + 1, text);
if (nextLine) {
result.push(`${getPrintableLineNumber(realStartLine + 1)} ${gray(nextLine)}`);
}

// Force a new line at the end
result.push('');

return result.join('\n');
}

function generateString(str: string, len: number): string {
return Array.from({ length: len }, () => str).join('');
}

function getStringForSeverity(diag: Diagnostic): string {
switch (diag.severity) {
case DiagnosticSeverity.Error:
return 'Error';
case DiagnosticSeverity.Warning:
return 'Warning';
case DiagnosticSeverity.Hint:
return 'Hint';
default:
return 'Unknown';
}
}

function getColorForSeverity(diag: Diagnostic, text: string): string {
switch (diag.severity) {
case DiagnosticSeverity.Error:
return red(text);
case DiagnosticSeverity.Warning:
return yellow(text);
case DiagnosticSeverity.Hint:
return gray(text);
default:
return text;
}
}

function getBackgroundForSeverity(diag: Diagnostic, text: string): string {
switch (diag.severity) {
case DiagnosticSeverity.Error:
return bgRed(white(text));
case DiagnosticSeverity.Warning:
return bgYellow(white(text));
case DiagnosticSeverity.Hint:
return bgWhite(black(text));
default:
return text;
}
}

function getPrintableLineNumber(line: number): string {
return bgWhite(black(line.toString().padStart(2, '0')));
}

function getLine(line: number, text: string): string {
return text
.substring(
offsetAt({ line, character: 0 }, text),
offsetAt({ line, character: Number.MAX_SAFE_INTEGER }, text)
)
.replace(/\t/g, ' ')
.trimEnd();
}
2 changes: 1 addition & 1 deletion packages/astro/src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import preview from '../core/preview/index.js';
import { ASTRO_VERSION, createSafeError } from '../core/util.js';
import * as event from '../events/index.js';
import { eventConfigError, eventError, telemetry } from '../events/index.js';
import { check } from './check.js';
import { check } from './check/index.js';
import { openInBrowser } from './open.js';
import * as telemetryHandler from './telemetry.js';

Expand Down
25 changes: 25 additions & 0 deletions packages/astro/test/cli.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,31 @@ describe('astro cli', () => {
expect(proc.stdout).to.include('Complete');
});

it('astro check no errors', async () => {
let proc = undefined;
const projectRootURL = new URL('./fixtures/astro-check-no-errors/', import.meta.url);
try {
proc = await cli('check', '--root', fileURLToPath(projectRootURL));
} catch (err) {}

expect(proc?.stdout).to.include('0 errors');
});

it('astro check has errors', async () => {
let stdout = undefined;
const projectRootURL = new URL('./fixtures/astro-check-errors/', import.meta.url);

// When `astro check` finds errors, it returns an error code. As such, we need to wrap this
// in a try/catch because otherwise Mocha will always report this test as a fail
try {
await cli('check', '--root', fileURLToPath(projectRootURL));
} catch (err) {
stdout = err.toString();
}

expect(stdout).to.include('1 error');
});

it('astro dev welcome', async () => {
const pkgURL = new URL('../package.json', import.meta.url);
const pkgVersion = await fs.readFile(pkgURL, 'utf8').then((data) => JSON.parse(data).version);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { defineConfig } from 'astro/config';

// https://astro.build/config
export default defineConfig({
integrations: [],
});
8 changes: 8 additions & 0 deletions packages/astro/test/fixtures/astro-check-errors/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "@test/astro-check-errors",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
}
}
Loading