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

Commit

Permalink
Code quality polish (#936)
Browse files Browse the repository at this point in the history
  • Loading branch information
sebmck committed Aug 5, 2020
1 parent aff295d commit 9494b1b
Show file tree
Hide file tree
Showing 250 changed files with 3,568 additions and 3,994 deletions.
7 changes: 1 addition & 6 deletions .editorconfig
Expand Up @@ -9,11 +9,6 @@ indent_style = tab
indent_size = 2

# YAML doesn't support hard tabs 🙃
[{*.yml}]
indent_style = space
indent_size = 2

# Templates that will be weird with hard tabs in the website editor
[.github/**.md]
[{*.yml, .github/**.md}]
indent_style = space
indent_size = 2
2 changes: 1 addition & 1 deletion internal/ast/js/modules/JSExportDefaultDeclaration.ts
Expand Up @@ -10,10 +10,10 @@ import {
JSClassDeclaration,
JSFunctionDeclaration,
NodeBaseWithComments,
TSDeclareFunction,
TSInterfaceDeclaration,
} from "@internal/ast";
import {createBuilder} from "../../utils";
import {TSDeclareFunction} from "../typescript/TSDeclareFunction";

export interface JSExportDefaultDeclaration extends NodeBaseWithComments {
readonly type: "JSExportDefaultDeclaration";
Expand Down
6 changes: 0 additions & 6 deletions internal/ast/js/unions.ts
Expand Up @@ -19,10 +19,6 @@ export type AnyTSTypeElement =

export type AnyTSModuleReference = AnyTSEntityName | n.TSExternalModuleReference;

export type AnyTSFunctionOrConstructorType =
| n.TSFunctionType
| n.TSConstructorType;

export type JSObjectProperties = Array<
n.JSObjectProperty | n.JSObjectMethod | n.JSSpreadProperty
>;
Expand Down Expand Up @@ -79,7 +75,6 @@ export type AnyJSAuxiliary =
| n.JSImportSpecifier
| n.JSImportDefaultSpecifier
| n.JSImportNamespaceSpecifier
| n.JSExportLocalSpecifier
| n.JSExportNamespaceSpecifier
| n.JSDirective
| n.JSInterpreterDirective
Expand Down Expand Up @@ -253,7 +248,6 @@ export type AnyTSLiteralTypeAnnotation =
export type AnyTSKeywordTypeAnnotation =
| n.TSAnyKeywordTypeAnnotation
| n.TSBooleanKeywordTypeAnnotation
| n.TSNumberKeywordTypeAnnotation
| n.TSStringKeywordTypeAnnotation
| n.TSBigIntKeywordTypeAnnotation
| n.TSNeverKeywordTypeAnnotation
Expand Down
32 changes: 14 additions & 18 deletions internal/ast/utils.ts
Expand Up @@ -58,29 +58,25 @@ export function createQuickBuilder<
opts: CreateBuilderOptions<Node>,
): QuickBuilder<Node, Node[QuickKey]> {
declareBuilder(type, opts);

return new QuickBuilder(type, opts.visitorKeys, quickKey);
return new QuickBuilder(type, quickKey);
}

export function createBuilder<Node extends AnyNode>(
type: string,
opts: CreateBuilderOptions<Node>,
): Builder<Node> {
declareBuilder(type, opts);

return new Builder(type, opts.visitorKeys);
return new Builder(type);
}

class Builder<Node extends AnyNode> {
constructor(type: string, visitorKeys: VisitorKeys<Node>) {
constructor(type: string) {
this.type = type;
this.visitorKeys = visitorKeys;
}

type: string;
visitorKeys: VisitorKeys<Node>;
private type: string;

create(opts: Omit<Node, "type">, inheritNode?: AnyNode): Node {
public create(opts: Omit<Node, "type">, inheritNode?: AnyNode): Node {
// @ts-ignore
return Object.freeze({
loc: inheritNode === undefined ? undefined : inheritLoc(inheritNode),
Expand All @@ -89,7 +85,7 @@ class Builder<Node extends AnyNode> {
});
}

assert(res: undefined | AnyNodes): Node {
public assert(res: undefined | AnyNodes): Node {
if (res === undefined) {
throw new Error(`Expected ${this.type} Node but got undefined`);
}
Expand All @@ -106,18 +102,18 @@ class Builder<Node extends AnyNode> {
}

class QuickBuilder<Node extends AnyNode, Arg> extends Builder<Node> {
constructor(
type: string,
visitorKeys: VisitorKeys<Node>,
quickKey: keyof Node,
) {
super(type, visitorKeys);
constructor(type: string, quickKey: keyof Node) {
super(type);
this.quickKey = quickKey;
}

quickKey: keyof Node;
private quickKey: keyof Node;

quick(arg: Arg, opts?: Partial<Omit<Node, "type">>, inheritNode?: Node): Node {
public quick(
arg: Arg,
opts?: Partial<Omit<Node, "type">>,
inheritNode?: Node,
): Node {
const node = ({
...opts,
[this.quickKey]: arg,
Expand Down
103 changes: 44 additions & 59 deletions internal/cli-diagnostics/DiagnosticsPrinter.ts
Expand Up @@ -165,27 +165,28 @@ export default class DiagnosticsPrinter extends Error {
this.onFooterPrintCallbacks = [];
}

options: DiagnosticsPrinterOptions;
reporter: Reporter;
processor: DiagnosticsProcessor;
onFooterPrintCallbacks: Array<{
public processor: DiagnosticsProcessor;
public flags: DiagnosticsPrinterFlags;

private options: DiagnosticsPrinterOptions;
private reporter: Reporter;
private onFooterPrintCallbacks: Array<{
callback: FooterPrintCallback;
after: boolean;
}>;
flags: DiagnosticsPrinterFlags;
cwd: AbsoluteFilePath;
fileReaders: Array<DiagnosticsFileReaders>;
hasTruncatedDiagnostics: boolean;
missingFileSources: UnknownFilePathSet;
fileSources: DiagnosticsPrinterFileSources;
fileMtimes: DiagnosticsPrinterFileMtimes;

displayedCount: number;
problemCount: number;
filteredCount: number;
truncatedCount: number;

createFilePath(filename: string): UnknownFilePath {
private cwd: AbsoluteFilePath;
private fileReaders: Array<DiagnosticsFileReaders>;
private hasTruncatedDiagnostics: boolean;
private missingFileSources: UnknownFilePathSet;
private fileSources: DiagnosticsPrinterFileSources;
private fileMtimes: DiagnosticsPrinterFileMtimes;

private displayedCount: number;
private problemCount: number;
private filteredCount: number;
private truncatedCount: number;

public createFilePath(filename: string): UnknownFilePath {
const {normalizePosition} = this.reporter.markupOptions;

if (normalizePosition === undefined) {
Expand All @@ -197,32 +198,18 @@ export default class DiagnosticsPrinter extends Error {
}
}

throwIfAny() {
if (this.hasDiagnostics()) {
throw this;
}
}

hasDiagnostics(): boolean {
return this.processor.hasDiagnostics();
}

getDisplayedProblemsCount() {
private getDisplayedProblemsCount() {
return this.problemCount - this.filteredCount;
}

shouldTruncate(): boolean {
if (
private shouldTruncate(): boolean {
return (
!this.flags.showAllDiagnostics &&
this.displayedCount > this.flags.maxDiagnostics
) {
return true;
} else {
return false;
}
);
}

shouldIgnore(diag: Diagnostic): boolean {
private shouldIgnore(diag: Diagnostic): boolean {
const {grep, inverseGrep} = this.flags;

// An empty grep pattern means show everything
Expand All @@ -242,7 +229,7 @@ export default class DiagnosticsPrinter extends Error {
}

// Only highlight if we have a reporter stream enabled that isn't format: "none"
shouldHighlight(): boolean {
public shouldHighlight(): boolean {
for (const {stream} of this.reporter.getStreamHandles()) {
if (stream.format !== "none") {
return true;
Expand All @@ -251,7 +238,9 @@ export default class DiagnosticsPrinter extends Error {
return false;
}

async addFileSource(dep: ChangeFileDependency | ReferenceFileDependency) {
private async addFileSource(
dep: ChangeFileDependency | ReferenceFileDependency,
) {
const path = dep.path.assertAbsolute();

let mtime;
Expand Down Expand Up @@ -300,7 +289,7 @@ export default class DiagnosticsPrinter extends Error {
}
}

getDependenciesFromDiagnostics(
private getDependenciesFromDiagnostics(
diagnostics: Diagnostics,
): Array<FileDependency> {
const deps: Array<FileDependency> = [];
Expand Down Expand Up @@ -370,11 +359,7 @@ export default class DiagnosticsPrinter extends Error {
for (const {filename, line, column, sourceText} of item.frames) {
if (filename !== undefined) {
const path = this.createFilePath(filename);
if (
filename !== undefined &&
line !== undefined &&
column !== undefined
) {
if (line !== undefined && column !== undefined) {
deps.push({
type: "reference",
path,
Expand Down Expand Up @@ -441,7 +426,7 @@ export default class DiagnosticsPrinter extends Error {
return Array.from(depsMap.values());
}

async fetchFileSources(diagnostics: Diagnostics) {
public async fetchFileSources(diagnostics: Diagnostics) {
for (const dep of this.getDependenciesFromDiagnostics(diagnostics)) {
const {path} = dep;
if (!path.isAbsolute()) {
Expand All @@ -455,7 +440,7 @@ export default class DiagnosticsPrinter extends Error {
}
}

async print() {
public async print() {
await this.wrapError(
"root",
async () => {
Expand All @@ -466,7 +451,7 @@ export default class DiagnosticsPrinter extends Error {
);
}

async wrapError(reason: string, callback: () => Promise<void>) {
private async wrapError(reason: string, callback: () => Promise<void>) {
const {reporter} = this;
try {
await callback();
Expand All @@ -486,7 +471,7 @@ export default class DiagnosticsPrinter extends Error {
}
}

async printDiagnostics(diagnostics: Diagnostics) {
private async printDiagnostics(diagnostics: Diagnostics) {
const {reporter} = this;
const restoreRedirect = reporter.redirectOutToErr(true);

Expand All @@ -504,7 +489,7 @@ export default class DiagnosticsPrinter extends Error {
reporter.redirectOutToErr(restoreRedirect);
}

getOutdatedFiles(diag: Diagnostic): UnknownFilePathSet {
public getOutdatedFiles(diag: Diagnostic): UnknownFilePathSet {
let outdatedFiles: UnknownFilePathSet = new UnknownFilePathSet();
for (const {
path,
Expand All @@ -523,7 +508,7 @@ export default class DiagnosticsPrinter extends Error {
return outdatedFiles;
}

printAuxiliaryDiagnostic(diag: Diagnostic) {
private printAuxiliaryDiagnostic(diag: Diagnostic) {
const {description: {message}, location: {start, filename}} = diag;

switch (this.flags.auxiliaryDiagnosticFormat) {
Expand Down Expand Up @@ -562,7 +547,7 @@ export default class DiagnosticsPrinter extends Error {
}
}

printDiagnostic(diag: Diagnostic) {
public printDiagnostic(diag: Diagnostic) {
const {reporter} = this;
const {start, end, filename} = diag.location;
let advice = [...diag.description.advice];
Expand Down Expand Up @@ -713,7 +698,7 @@ export default class DiagnosticsPrinter extends Error {
});
}

filterDiagnostics(): Diagnostics {
private filterDiagnostics(): Diagnostics {
const diagnostics = this.processor.getDiagnostics();
const filteredDiagnostics: Diagnostics = [];

Expand All @@ -733,7 +718,7 @@ export default class DiagnosticsPrinter extends Error {
return filteredDiagnostics;
}

inject(title: StaticMarkup, printer: DiagnosticsPrinter) {
public inject(title: StaticMarkup, printer: DiagnosticsPrinter) {
this.processor.addDiagnostics(printer.processor.getDiagnostics());

const {onFooterPrintCallbacks} = printer;
Expand All @@ -759,15 +744,15 @@ export default class DiagnosticsPrinter extends Error {
);
}

onFooterPrint(callback: FooterPrintCallback, after: boolean = false) {
public onFooterPrint(callback: FooterPrintCallback, after: boolean = false) {
this.onFooterPrintCallbacks.push({callback, after});
}

hasProblems(): boolean {
public hasProblems(): boolean {
return this.problemCount > 0;
}

async footer() {
public async footer() {
await this.wrapError(
"footer",
async () => {
Expand Down Expand Up @@ -825,7 +810,7 @@ export default class DiagnosticsPrinter extends Error {
);
}

showBanner(banner: RawBanner) {
private showBanner(banner: RawBanner) {
for (const {stream} of this.reporter.getStreamHandles()) {
if (stream.format !== "ansi") {
continue;
Expand Down Expand Up @@ -968,7 +953,7 @@ export default class DiagnosticsPrinter extends Error {
}
}

footerError() {
private footerError() {
const {reporter, filteredCount} = this;

const displayableProblems = this.getDisplayedProblemsCount();
Expand Down
10 changes: 9 additions & 1 deletion internal/cli-diagnostics/buildCodeFrame.ts
Expand Up @@ -304,7 +304,15 @@ export default function buildCodeFrame(
if (line !== undefined) {
const text = line[0].slice(ob1Get0(start.column), ob1Get0(end.column));
if (cleanEquivalentString(text) === cleanEquivalentString(markerMessage)) {
markerMessage = markup``;
for (const selection of formattedLines) {
if (
selection !== undefined &&
selection.marker !== undefined &&
selection.marker.message === markerMessage
) {
selection.marker.message = markup``;
}
}
}
}
}
Expand Down

0 comments on commit 9494b1b

Please sign in to comment.