Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
Merge pull request #163 from facebookexperimental/categories
Browse files Browse the repository at this point in the history
diagnostics: Dedicated diagnostics string literal union
  • Loading branch information
sebmck authored Mar 10, 2020
2 parents fee494f + a359084 commit 2723f2d
Show file tree
Hide file tree
Showing 76 changed files with 553 additions and 204 deletions.
2 changes: 1 addition & 1 deletion packages/@romejs/cli-flags/Parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ export default class Parser<T> {
},

context: {
category: 'cli-flags',
category: 'flags/invalid',
getOriginalValue: (keys: ConsumePath) => {
return flags[keys[0]];
},
Expand Down
20 changes: 16 additions & 4 deletions packages/@romejs/codec-js-manifest/dependencies.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,22 @@ import {parseDependencyPattern} from './dependencies';
import {consumeUnknown} from '@romejs/consume';

test('can parse npm dependency patterns', t => {
t.snapshot(parseDependencyPattern(consumeUnknown('npm:foo'), false));
t.snapshot(parseDependencyPattern(consumeUnknown('npm:@foo/bar'), false));
t.snapshot(parseDependencyPattern(consumeUnknown('npm:[email protected]'), false));
t.snapshot(
parseDependencyPattern(consumeUnknown('npm:@foo/[email protected]'), false),
parseDependencyPattern(consumeUnknown('npm:foo', 'parse/json'), false),
);
t.snapshot(
parseDependencyPattern(consumeUnknown('npm:@foo/bar', 'parse/json'), false),
);
t.snapshot(
parseDependencyPattern(
consumeUnknown('npm:[email protected]', 'parse/json'),
false,
),
);
t.snapshot(
parseDependencyPattern(
consumeUnknown('npm:@foo/[email protected]', 'parse/json'),
false,
),
);
});
4 changes: 2 additions & 2 deletions packages/@romejs/codec-js-regexp/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export const createRegExpParser = createParser(
ParserCore =>
class RegExpParser extends ParserCore<Tokens, void> {
constructor(opts: RegExpParserOptions) {
super(opts, '@romejs/codec-js-regexp');
super(opts, 'parse/regex');
this.diagnostics = [];
this.unicode = opts.unicode;
}
Expand Down Expand Up @@ -707,7 +707,7 @@ export const createRegExpParser = createParser(
const quantified: RegExpQuantified = {
type: 'RegExpQuantified',
loc: this.finishLoc(start),
item: target,
target: target,
lazy,
...quantifier,
};
Expand Down
22 changes: 12 additions & 10 deletions packages/@romejs/codec-json/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import {DiagnosticPointer} from '@romejs/diagnostics';
import {DiagnosticPointer, DiagnosticCategory} from '@romejs/diagnostics';
import {
JSONParserResult,
JSONParserOptions,
Expand Down Expand Up @@ -113,7 +113,7 @@ export default createParser(
ParserCore =>
class JSONParser extends ParserCore<Tokens, void> {
constructor(opts: JSONParserOptions) {
super(opts, '@romejs/codec-json');
super(opts, 'parse/json');
this.options = opts;
this.ignoreWhitespaceTokens = true;

Expand All @@ -123,16 +123,18 @@ export default createParser(
this.pathKeys = [];
this.paths = new Map();
this.pathToComments = new Map();
this.consumeCategory =
opts.consumeCategory === undefined ? 'json' : opts.consumeCategory;
this.consumeDiagnosticCategory =
opts.consumeDiagnosticCategory === undefined
? 'parse/json'
: opts.consumeDiagnosticCategory;
}

pathToComments: PathToComments;
hasExtensions: boolean;
pathKeys: ConsumePath;
paths: Map<string, PathInfo>;
options: JSONParserOptions;
consumeCategory: string;
consumeDiagnosticCategory: DiagnosticCategory;

getPathInfo(path: ConsumePath): undefined | PathInfo {
return this.paths.get(path.join('.'));
Expand Down Expand Up @@ -761,8 +763,8 @@ export default createParser(
const value = JSON.parse(this.input);

// Lazy parse when we need location information
let context: undefined | ConsumeContext;
const getContext = (): ConsumeContext => {
let context: undefined | Required<ConsumeContext>;
const getContext = (): Required<ConsumeContext> => {
if (context === undefined) {
const res = this._parse();
context = res.context;
Expand All @@ -774,7 +776,7 @@ export default createParser(

return {
context: {
category: this.consumeCategory,
category: this.consumeDiagnosticCategory,
getOriginalValue(path) {
return getContext().getOriginalValue(path);
},
Expand Down Expand Up @@ -819,8 +821,8 @@ export default createParser(

this.finalize();

const context: ConsumeContext = {
category: this.consumeCategory,
const context: Required<ConsumeContext> = {
category: this.consumeDiagnosticCategory,

getDiagnosticPointer: (
keys: ConsumePath,
Expand Down
5 changes: 3 additions & 2 deletions packages/@romejs/codec-json/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ import {
ParserOptions,
} from '@romejs/parser-core';
import {ConsumeContext} from '@romejs/consume';
import {DiagnosticCategory} from '@romejs/diagnostics';

export type JSONParserOptions = ParserOptions & {
consumeCategory?: string;
consumeDiagnosticCategory?: DiagnosticCategory;
};

export type PathComments = {
Expand All @@ -38,7 +39,7 @@ export type Comments = Array<BlockComment | LineComment>;

export type JSONParserResult = {
value: JSONValue;
context: ConsumeContext;
context: Required<ConsumeContext>;
};

export type Tokens = BaseTokens & {
Expand Down
2 changes: 1 addition & 1 deletion packages/@romejs/codec-semver/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const createSemverParser = createParser(
ParserCore =>
class SemverParser extends ParserCore<Tokens, void> {
constructor({loose, ...opts}: SemverParserOptions, mode: ParseMode) {
super(opts, 'semver-parser');
super(opts, 'parse/semver');
this.input = this.input.trimRight();
this.mode = mode;
this.loose = loose === undefined ? false : loose;
Expand Down
2 changes: 1 addition & 1 deletion packages/@romejs/codec-spdx-license/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ const createSPDXLicenseParser = createParser(
ParserCore =>
class SPDXLicenseParser extends ParserCore<Tokens, void> {
constructor(opts: SPDXLicenseParserOptions) {
super(opts, 'spdx-license');
super(opts, 'parse/spdxLicense');
this.loose = opts.loose === true;
}

Expand Down
4 changes: 3 additions & 1 deletion packages/@romejs/codec-url/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ export type ConsumableUrl = {
export function consumeUrl(rawUrl: string): ConsumableUrl {
const parts = url.parse(rawUrl, true);

const query = consumeUnknown({...parts.query});
const query = consumeUnknown({...parts.query}, 'parse/url/query');

const path = consume({
value: parts.pathname,
context: {
category: 'parse/url',

getDiagnosticPointer() {
return {
language: 'url',
Expand Down
2 changes: 1 addition & 1 deletion packages/@romejs/codec-watchman/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export class WatchmanClient {
const bunser = new BunserBuf();

bunser.valueEvent.subscribe(obj => {
this.processResponse(consumeUnknown(obj));
this.processResponse(consumeUnknown(obj, 'parse/json'));
});

socket.on('data', chunk => {
Expand Down
19 changes: 16 additions & 3 deletions packages/@romejs/consume/Consumer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
DiagnosticsError,
DiagnosticPointer,
getDiagnosticsFromError,
DiagnosticCategory,
} from '@romejs/diagnostics';
import {UnknownObject} from '@romejs/typescript-helpers';
import {
Expand Down Expand Up @@ -53,6 +54,7 @@ import {
} from '@romejs/path';

type UnexpectedConsumerOptions = {
category?: DiagnosticCategory;
loc?: SourceLocation;
target?: ConsumeSourceLocationRequestTarget;
advice?: PartialDiagnosticAdvice;
Expand Down Expand Up @@ -186,11 +188,16 @@ export default class Consumer {
getDiagnosticPointer(
target: ConsumeSourceLocationRequestTarget = 'all',
): undefined | DiagnosticPointer {
const {getDiagnosticPointer} = this.context;
if (getDiagnosticPointer === undefined) {
return undefined;
}

const {forceDiagnosticTarget} = this;
if (forceDiagnosticTarget !== undefined) {
target = forceDiagnosticTarget;
}
return this.context.getDiagnosticPointer(this.keyPath, target);
return getDiagnosticPointer(this.keyPath, target);
}

getLocation(target?: ConsumeSourceLocationRequestTarget): SourceLocation {
Expand Down Expand Up @@ -254,7 +261,12 @@ export default class Consumer {
}

hasChangedFromSource(): boolean {
const originalValue = this.context.getOriginalValue(this.keyPath);
const {getOriginalValue} = this.context;
if (getOriginalValue === undefined) {
return false;
}

const originalValue = getOriginalValue(this.keyPath);
return !this.wasInSource() || this.value !== originalValue;
}

Expand Down Expand Up @@ -357,7 +369,8 @@ export default class Consumer {
}

const diagnostic: PartialDiagnostic = {
category: this.context.category,
category:
opts.category === undefined ? this.context.category : opts.category,
filename: this.filename,
message: msg,
...loc,
Expand Down
36 changes: 12 additions & 24 deletions packages/@romejs/consume/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,51 +5,39 @@
* LICENSE file in the root directory of this source tree.
*/

import {ConsumerOptions, ConsumeContext} from './types';
import {ConsumerOptions} from './types';
import Consumer from './Consumer';
import {RequiredProps} from '@romejs/typescript-helpers';
import {DiagnosticCategory} from '@romejs/diagnostics';

export const EMPTY_CONSUME_CONTEXT: ConsumeContext = {
category: 'unknown',

getOriginalValue() {
return undefined;
},

getDiagnosticPointer() {
return undefined;
},
};

const EMPTY_CONSUME_OPTIONS: ConsumerOptions = {
const EMPTY_CONSUME_OPTIONS: Omit<ConsumerOptions, 'context'> = {
propertyMetadata: undefined,
value: undefined,
handleUnexpectedDiagnostic: undefined,
onDefinition: undefined,
filePath: undefined,
context: EMPTY_CONSUME_CONTEXT,
objectPath: [],
parent: undefined,
};

export function consume(
opts: Partial<Omit<ConsumerOptions, 'value' | 'context'>> & {
value: unknown;
context?: Partial<ConsumeContext>;
},
opts: RequiredProps<Partial<ConsumerOptions>, 'context'>,
): Consumer {
return new Consumer({
...EMPTY_CONSUME_OPTIONS,
...opts,
context: {
...EMPTY_CONSUME_CONTEXT,
...opts.context,
},
});
}

export function consumeUnknown(value: unknown): Consumer {
export function consumeUnknown(
value: unknown,
category: DiagnosticCategory,
): Consumer {
return new Consumer({
...EMPTY_CONSUME_OPTIONS,
context: {
category,
},
value,
});
}
Expand Down
12 changes: 8 additions & 4 deletions packages/@romejs/consume/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
* LICENSE file in the root directory of this source tree.
*/

import {DiagnosticPointer, PartialDiagnostic} from '@romejs/diagnostics';
import {
DiagnosticPointer,
PartialDiagnostic,
DiagnosticCategory,
} from '@romejs/diagnostics';
import Consumer from './Consumer';
import {UnknownFilePath} from '@romejs/path';
import {Number0, Number1} from '@romejs/ob1';
Expand All @@ -23,12 +27,12 @@ export type ConsumeSourceLocationRequestTarget =
| 'inner-value';

export type ConsumeContext = {
category: string;
getDiagnosticPointer: (
category: DiagnosticCategory;
getDiagnosticPointer?: (
keys: ConsumePath,
target: ConsumeSourceLocationRequestTarget,
) => undefined | DiagnosticPointer;
getOriginalValue: (path: ConsumePath) => unknown;
getOriginalValue?: (path: ConsumePath) => unknown;
};

export type ConsumePropertyMetadata = {
Expand Down
4 changes: 3 additions & 1 deletion packages/@romejs/core/client/ClientRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ export default class ClientRequest {

let flags;
if (localCommand.defineFlags !== undefined) {
flags = localCommand.defineFlags(consumeUnknown(query.commandFlags));
flags = localCommand.defineFlags(
consumeUnknown(query.commandFlags, 'flags/invalid'),
);
}

const success = await localCommand.callback(this, flags);
Expand Down
2 changes: 1 addition & 1 deletion packages/@romejs/core/client/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ localCommands.set('run', {
return false;
}

const data = consumeUnknown(res.data);
const data = consumeUnknown(res.data, 'parse/json');

if (data.exists()) {
const type = data.get('type').asString();
Expand Down
2 changes: 1 addition & 1 deletion packages/@romejs/core/common/utils/executeMain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export default async function executeMain(

const syntaxError: PartialDiagnostic = {
message: err.message,
category: 'syntaxError',
category: 'v8/syntaxError',
start: pos,
end: pos,
filename,
Expand Down
4 changes: 2 additions & 2 deletions packages/@romejs/core/master/Master.ts
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@ export default class Master {
},
objectPath: [],
context: {
category: 'cli-flags',
category: 'flags/invalid',

getOriginalValue: () => {
return undefined;
Expand Down Expand Up @@ -858,7 +858,7 @@ export default class Master {
message: 'Error captured and converted into a diagnostic',
});
const errorDiag = deriveDiagnosticFromError({
category: 'internalError',
category: 'internalError/request',
error: err,
});
printer.addDiagnostic({
Expand Down
Loading

0 comments on commit 2723f2d

Please sign in to comment.