Skip to content

Commit

Permalink
refactor: consolidate diagnostics funcs into single file (#416)
Browse files Browse the repository at this point in the history
* refactor: consolidate `diagnostics` funcs into single file

- move `IDiagnostic` and `convertDiagnostic` from `tscache` to `print-diagnostics`, then rename it to `diagnostics.ts`
  - the diagnostic funcs being in `tscache` always felt like a strange place for them
    - especially when `parse-tsconfig` or `print-diagnostics` would import them from `tscache`, which sounded like an unrelated file

- may want to move `diagnostics-format-host` into this file as well, as it is _only_ used in this file
  - leaving as is for now, limiting this change to a smaller one

* test: add unit test for `convertDiagnostic`

- this was previously only covered in integration tests
  - since unit tests don't currently touch `index` or `tscache`, and `convertDiagnostic` was previously in `tscache`
    - another reason why consolidating these functions into one file made sense

* fix(diagnostics): use `formatHost.getNewLine()` instead of `\n`

- since new lines are host OS specific

- this fixes the `convertDiagnostic` test on Windows too, where it was failing
  • Loading branch information
agilgur5 authored Sep 12, 2022
1 parent e98e0ed commit ba26293
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 45 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,5 +73,5 @@ A useful resource as you dive deeper are the [unit tests](__tests__/). They're g
1. At this point, you may be ready to read the more complicated bits of [`index`](src/index.ts) in detail and see how it interacts with the other modules.
- The [integration tests](__tests__/integration/) could be useful to review at this point as well.
1. Once you're pretty familiar with `index`, you can dive into some of the cache code in [`tscache`](src/tscache.ts) and [`rollingcache`](src/rollingcache.ts).
1. And finally, you can see some of the Rollup logging nuances in [`context`](src/context.ts) and then the TS logging nuances in [`print-diagnostics`](src/print-diagnostics.ts), and [`diagnostics-format-host`](src/diagnostics-format-host.ts)
1. And finally, you can see some of the Rollup logging nuances in [`context`](src/context.ts) and then the TS logging nuances in [`diagnostics`](src/diagnostics.ts), and [`diagnostics-format-host`](src/diagnostics-format-host.ts)
- While these are necessary to the implementation, they are fairly ancillary to understanding and working with the codebase.
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,35 @@ import { red } from "colors/safe";

import { makeContext } from "./fixtures/context";
import { setTypescriptModule } from "../src/tsproxy";
import { printDiagnostics } from "../src/print-diagnostics";
import { formatHost } from "../src/diagnostics-format-host";
import { convertDiagnostic, printDiagnostics } from "../src/diagnostics";

setTypescriptModule(ts);

const tsDiagnostic = {
file: undefined,
start: undefined,
length: undefined,
messageText: "Compiler option 'include' requires a value of type Array.",
category: ts.DiagnosticCategory.Error,
code: 5024,
reportsUnnecessary: undefined,
reportsDeprecated: undefined,
};

const diagnostic = {
flatMessage: "Compiler option 'include' requires a value of type Array.",
formatted: "\x1B[91merror\x1B[0m\x1B[90m TS5024: \x1B[0mCompiler option 'include' requires a value of type Array.\n",
formatted: `\x1B[91merror\x1B[0m\x1B[90m TS5024: \x1B[0mCompiler option 'include' requires a value of type Array.${formatHost.getNewLine()}`,
category: ts.DiagnosticCategory.Error,
code: 5024,
type: 'config'
type: "config",
};

test("print-diagnostics - categories", () => {
test("convertDiagnostic", () => {
expect(convertDiagnostic("config", [tsDiagnostic])).toStrictEqual([diagnostic]);
});

test("printDiagnostics - categories", () => {
const context = makeContext();

printDiagnostics(context, [diagnostic]);
Expand All @@ -38,7 +54,7 @@ test("print-diagnostics - categories", () => {
expect(context.debug).toBeCalledTimes(0);
});

test("print-diagnostics - formatting / style", () => {
test("printDiagnostics - formatting / style", () => {
const context = makeContext();
const category = "error"; // string version

Expand Down
35 changes: 34 additions & 1 deletion src/print-diagnostics.ts → src/diagnostics.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,41 @@
import * as tsTypes from "typescript";
import { red, white, yellow } from "colors/safe";

import { tsModule } from "./tsproxy";
import { RollupContext } from "./context";
import { IDiagnostics } from "./tscache";
import { formatHost } from "./diagnostics-format-host";

export interface IDiagnostics
{
flatMessage: string;
formatted: string;
fileLine?: string;
category: tsTypes.DiagnosticCategory;
code: number;
type: string;
}

export function convertDiagnostic(type: string, data: tsTypes.Diagnostic[]): IDiagnostics[]
{
return data.map((diagnostic) =>
{
const entry: IDiagnostics = {
flatMessage: tsModule.flattenDiagnosticMessageText(diagnostic.messageText, formatHost.getNewLine()),
formatted: tsModule.formatDiagnosticsWithColorAndContext(data, formatHost),
category: diagnostic.category,
code: diagnostic.code,
type,
};

if (diagnostic.file && diagnostic.start !== undefined)
{
const { line, character } = diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start);
entry.fileLine = `${diagnostic.file.fileName}(${line + 1},${character + 1})`;
}

return entry;
});
}

export function printDiagnostics(context: RollupContext, diagnostics: IDiagnostics[], pretty = true): void
{
Expand Down
4 changes: 2 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import findCacheDir from "find-cache-dir";

import { RollupContext, VerbosityLevel } from "./context";
import { LanguageServiceHost } from "./host";
import { TsCache, convertDiagnostic, convertEmitOutput, getAllReferences, ICode } from "./tscache";
import { TsCache, convertEmitOutput, getAllReferences, ICode } from "./tscache";
import { tsModule, setTypescriptModule } from "./tsproxy";
import { IOptions } from "./ioptions";
import { parseTsConfig } from "./parse-tsconfig";
import { printDiagnostics } from "./print-diagnostics";
import { convertDiagnostic, printDiagnostics } from "./diagnostics";
import { TSLIB, TSLIB_VIRTUAL, tslibSource, tslibVersion } from "./tslib";
import { createFilter } from "./get-options-overrides";

Expand Down
3 changes: 1 addition & 2 deletions src/parse-tsconfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ import * as _ from "lodash";

import { tsModule } from "./tsproxy";
import { RollupContext } from "./context";
import { printDiagnostics } from "./print-diagnostics";
import { convertDiagnostic } from "./tscache";
import { convertDiagnostic, printDiagnostics } from "./diagnostics";
import { getOptionsOverrides } from "./get-options-overrides";
import { IOptions } from "./ioptions";

Expand Down
35 changes: 1 addition & 34 deletions src/tscache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { RollupContext } from "./context";
import { RollingCache } from "./rollingcache";
import { ICache } from "./icache";
import { tsModule } from "./tsproxy";
import { formatHost } from "./diagnostics-format-host";
import { IDiagnostics, convertDiagnostic } from "./diagnostics";

export interface ICode
{
Expand All @@ -25,16 +25,6 @@ interface INodeLabel
dirty: boolean;
}

export interface IDiagnostics
{
flatMessage: string;
formatted: string;
fileLine?: string;
category: tsTypes.DiagnosticCategory;
code: number;
type: string;
}

interface ITypeSnapshot
{
id: string;
Expand Down Expand Up @@ -74,29 +64,6 @@ export function getAllReferences(importer: string, snapshot: tsTypes.IScriptSnap
}));
}

export function convertDiagnostic(type: string, data: tsTypes.Diagnostic[]): IDiagnostics[]
{
return data.map((diagnostic) =>
{
const entry: IDiagnostics =
{
flatMessage: tsModule.flattenDiagnosticMessageText(diagnostic.messageText, "\n"),
formatted: tsModule.formatDiagnosticsWithColorAndContext(data, formatHost),
category: diagnostic.category,
code: diagnostic.code,
type,
};

if (diagnostic.file && diagnostic.start !== undefined)
{
const { line, character } = diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start);
entry.fileLine = `${diagnostic.file.fileName}(${line + 1},${character + 1})`;
}

return entry;
});
}

export class TsCache
{
private cacheVersion = "9";
Expand Down

0 comments on commit ba26293

Please sign in to comment.