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 readability of source code #1889

Merged
merged 21 commits into from
Mar 20, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 4 additions & 2 deletions src/lib/application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
} from "./utils/component";
import { Options, BindOption } from "./utils";
import type { TypeDocOptions } from "./utils/options/declaration";
import { flatMap, unique } from "./utils/array";
import { unique } from "./utils/array";
import { ok } from "assert";
import {
DocumentationEntryPoint,
Expand Down Expand Up @@ -225,7 +225,9 @@ export class Application extends ChildableComponent<
`Converting with ${programs.length} programs ${entryPoints.length} entry points`
);

const errors = flatMap([...programs], ts.getPreEmitDiagnostics);
const errors = programs.flatMap((program) =>
ts.getPreEmitDiagnostics(program)
);
if (errors.length) {
this.logger.diagnostics(errors);
return;
Expand Down
26 changes: 8 additions & 18 deletions src/lib/converter/converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -392,31 +392,21 @@ export class Converter extends ChildableComponent<
this.excludeCache ??= createMinimatch(
this.application.options.getValue("exclude")
);
const cache = this.excludeCache;

for (const node of symbol.getDeclarations() ?? []) {
if (matchesAny(this.excludeCache, node.getSourceFile().fileName)) {
return true;
}
}

return false;
return (symbol.getDeclarations() ?? []).some((node) =>
matchesAny(cache, node.getSourceFile().fileName)
);
}

/** @internal */
isExternal(symbol: ts.Symbol) {
this.externalPatternCache ??= createMinimatch(this.externalPattern);
for (const node of symbol.getDeclarations() ?? []) {
if (
matchesAny(
this.externalPatternCache,
node.getSourceFile().fileName
)
) {
return true;
}
}
const cache = this.externalPatternCache;

return false;
return (symbol.getDeclarations() ?? []).some((node) =>
matchesAny(cache, node.getSourceFile().fileName)
);
}
}

Expand Down
3 changes: 1 addition & 2 deletions src/lib/converter/jsdoc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
ReflectionType,
SignatureReflection,
} from "../models";
import { flatMap } from "../utils/array";
import { getJsDocComment } from "./comments";
import type { Context } from "./context";
import { ConverterEvents } from "./converter-events";
Expand Down Expand Up @@ -166,6 +165,6 @@ function convertTemplateParameterNodes(
context: Context,
nodes: readonly ts.JSDocTemplateTag[] | undefined
) {
const params = flatMap(nodes ?? [], (tag) => tag.typeParameters);
const params = (nodes ?? []).flatMap((tag) => tag.typeParameters);
return convertTypeParameterNodes(context, params);
}
5 changes: 2 additions & 3 deletions src/lib/converter/plugins/CommentPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,8 @@ export class CommentPlugin extends ConverterComponent {
);
someRemoved.forEach((reflection) => {
reflection.sources = unique(
reflection.signatures!.reduce<SourceReference[]>(
(c, s) => c.concat(s.sources || []),
[]
reflection.signatures!.flatMap<SourceReference>(
(s) => s.sources ?? []
)
);
});
Expand Down
5 changes: 1 addition & 4 deletions src/lib/converter/plugins/ImplementsPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,10 +266,7 @@ export class ImplementsPlugin extends ConverterComponent {
}

// Need this because we re-use reflections for type literals.
if (
!reflection.parent ||
!reflection.parent.kindOf(ReflectionKind.ClassOrInterface)
) {
if (!reflection.parent?.kindOf(ReflectionKind.ClassOrInterface)) {
return;
}

Expand Down
6 changes: 2 additions & 4 deletions src/lib/converter/plugins/SourcePlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,8 @@ export class SourcePlugin extends ConverterComponent {
const project = context.project;
const home = project.directory;
project.files.forEach((file) => {
const reflections: DeclarationReflection[] = [];
file.reflections.forEach((reflection) => {
reflections.push(reflection);
});
const reflections: DeclarationReflection[] =
file.reflections.slice();

let directory = home;
const path = Path.dirname(file.fileName);
Expand Down
12 changes: 6 additions & 6 deletions src/lib/converter/utils/nodes.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as ts from "typescript";
import { flatMap } from "../../utils/array";

export function isNamedNode(node: ts.Node): node is ts.Node & {
name: ts.Identifier | ts.PrivateIdentifier | ts.ComputedPropertyName;
Expand All @@ -16,11 +15,12 @@ export function getHeritageTypes(
declarations: readonly (ts.ClassDeclaration | ts.InterfaceDeclaration)[],
kind: ts.SyntaxKind.ImplementsKeyword | ts.SyntaxKind.ExtendsKeyword
): ts.ExpressionWithTypeArguments[] {
const exprs = flatMap(declarations, (d) =>
flatMap(
d.heritageClauses?.filter((hc) => hc.token === kind) ?? [],
(hc) => hc.types as readonly ts.ExpressionWithTypeArguments[]
)
const exprs = declarations.flatMap((d) =>
(d.heritageClauses ?? [])
.filter((hc) => hc.token === kind)
.flatMap(
(hc) => hc.types as readonly ts.ExpressionWithTypeArguments[]
)
);

const seenTexts = new Set<string>();
Expand Down
10 changes: 4 additions & 6 deletions src/lib/models/comments/comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,25 +73,23 @@ export class Comment {
static combineDisplayParts(
parts: readonly CommentDisplayPart[] | undefined
): string {
const result: string[] = [];
let result = "";

for (const item of parts || []) {
switch (item.kind) {
case "text":
result.push(item.text);
break;
case "code":
result.push(item.text);
result += item.text;
break;
case "inline-tag":
result.push("{", item.tag, " ", item.text, "}");
result += `{${item.tag} ${item.text}}`;
break;
default:
assertNever(item);
}
}

return result.join("");
return result;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/lib/models/reflections/abstract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ export abstract class Reflection {
*/
getAlias(): string {
if (!this._alias) {
let alias = this.name.replace(/[^a-z0-9]/gi, "_");
let alias = this.name.replace(/\W/g, "_");
if (alias === "") {
alias = "reflection-" + this.id;
}
Expand Down
7 changes: 3 additions & 4 deletions src/lib/models/reflections/declaration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,10 +232,9 @@ export class DeclarationReflection extends ContainerReflection {
let result = super.toString();

if (this.typeParameters) {
const parameters: string[] = [];
this.typeParameters.forEach((parameter) => {
parameters.push(parameter.name);
});
const parameters: string[] = this.typeParameters.map(
(parameter) => parameter.name
);
result += "<" + parameters.join(", ") + ">";
}

Expand Down
5 changes: 2 additions & 3 deletions src/lib/models/reflections/signature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,8 @@ export class SignatureReflection extends Reflection {
let result = super.toString();

if (this.typeParameters) {
const parameters: string[] = [];
this.typeParameters.forEach((parameter) =>
parameters.push(parameter.name)
const parameters: string[] = this.typeParameters.map(
(parameter) => parameter.name
);
result += "<" + parameters.join(", ") + ">";
}
Expand Down
3 changes: 1 addition & 2 deletions src/lib/models/sources/directory.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import type { ReflectionGroup } from "../ReflectionGroup";
import type { SourceFile } from "./file";
import type { DeclarationReflection } from "..";
import { flatMap } from "../../utils/array";

/**
* Exposes information about a directory containing source files.
Expand Down Expand Up @@ -84,6 +83,6 @@ export class SourceDirectory {
* files of this directory.
*/
getAllReflections(): DeclarationReflection[] {
return flatMap(this.files, (file) => file.reflections);
return this.files.flatMap((file) => file.reflections);
}
}
3 changes: 1 addition & 2 deletions src/lib/output/plugins/JavascriptIndexPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ export class JavascriptIndexPlugin extends RendererComponent {
if (
!reflection.url ||
!reflection.name ||
reflection.flags.isExternal ||
reflection.name === ""
reflection.flags.isExternal
) {
continue;
}
Expand Down
35 changes: 11 additions & 24 deletions src/lib/output/plugins/LegendPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,31 +214,23 @@ export class LegendBuilder {

build(): LegendItem[][] {
const filteredLegend = completeLegend
.map((list) => {
return list.filter((item) => {
for (const classes of this._classesList) {
if (this.isArrayEqualToSet(item.classes, classes)) {
return true;
}
}
return false;
});
})
.map((list) =>
list.filter((item) =>
this._classesList.some((classes) =>
this.isArrayEqualToSet(item.classes, classes)
)
)
)
.filter((list) => list.length);

return filteredLegend;
}

registerCssClasses(classArray: string[]) {
let exists = false;
const items = classArray.filter((cls) => !ignoredClasses.has(cls));

for (const classes of this._classesList) {
if (this.isArrayEqualToSet(items, classes)) {
exists = true;
break;
}
}
const exists = this._classesList.some((classes) =>
this.isArrayEqualToSet(items, classes)
);

if (!exists) {
this._classesList.push(new Set(items));
Expand All @@ -250,12 +242,7 @@ export class LegendBuilder {
return false;
}

for (const value of a) {
if (!b.has(value)) {
return false;
}
}
return true;
return a.every((item) => b.has(item));
}
}

Expand Down
39 changes: 5 additions & 34 deletions src/lib/utils/array.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,11 @@ export function removeIfPresent<T>(arr: T[] | undefined, item: T) {
* @param predicate
*/
export function removeIf<T>(arr: T[], predicate: (item: T) => boolean) {
const indices = filterMap(arr, (item, index) =>
predicate(item) ? index : void 0
);
for (const index of indices.reverse()) {
arr.splice(index, 1);
for (let i = 0; i < arr.length; i++) {
if (predicate(arr[i])) {
arr.splice(i, 1);
i--;
}
}
}

Expand Down Expand Up @@ -116,17 +116,6 @@ export function partition<T>(
return [left, right];
}

/**
* Ensures the given item is an array.
* @param item
*/
export function toArray<T>(item: T | readonly T[] | undefined): T[] {
if (item === void 0) {
return [];
}
return Array.isArray(item) ? [...item] : [item];
}

export function* zip<T extends Iterable<any>[]>(
...args: T
): Iterable<{ [K in keyof T]: T[K] extends Iterable<infer U> ? U : T[K] }> {
Expand Down Expand Up @@ -156,21 +145,3 @@ export function filterMap<T, U>(

return result;
}

export function flatMap<T, U>(
arr: readonly T[],
fn: (item: T) => U | readonly U[] | undefined
): U[] {
const result: U[] = [];

for (const item of arr) {
const newItem = fn(item);
if (newItem instanceof Array) {
result.push(...newItem);
} else if (newItem != null) {
result.push(newItem);
}
}

return result;
}
2 changes: 1 addition & 1 deletion src/lib/utils/entry-point.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ function getEntryPointsForPaths(

entryLoop: for (const fileOrDir of inputFiles.map(normalizePath)) {
const toCheck = [fileOrDir];
if (!/\.[tj]sx?/.test(fileOrDir)) {
if (!/\.[tj]sx?$/.test(fileOrDir)) {
fb55 marked this conversation as resolved.
Show resolved Hide resolved
toCheck.push(
`${fileOrDir}/index.ts`,
`${fileOrDir}/index.tsx`,
Expand Down
4 changes: 2 additions & 2 deletions src/lib/utils/enum.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export function getEnumFlags<T extends number>(flags: T): T[] {
const result: T[] = [];
for (let i = 1; i <= flags; i *= 2) {
for (let i = 1; i <= flags; i <<= 1) {
if (flags & i) {
result.push(i as T);
}
Expand All @@ -11,7 +11,7 @@ export function getEnumFlags<T extends number>(flags: T): T[] {

// T & {} reduces inference priority
export function removeFlag<T extends number>(flag: T, remove: T & {}): T {
return ((flag ^ remove) & flag) as T;
return (flag & ~remove) as T;
}

export function hasAllFlags(flags: number, check: number): boolean {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/utils/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ function onApi(
options: OnApiOptions
): EventHandlers {
if (callback) {
const handlers = events[name] || (events[name] = []);
const handlers = (events[name] ||= []);
const context = options.context,
ctx = options.ctx,
listening = options.listening,
Expand Down
5 changes: 2 additions & 3 deletions src/lib/utils/package-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import { dirname, join, resolve } from "path";
import { existsSync } from "fs";
import { flatMap } from "./array";

import { readFile, glob } from "./fs";
import type { Logger } from "./loggers";
Expand Down Expand Up @@ -79,12 +78,12 @@ export function expandPackages(
// to the root of a workspace tree in our params and so we could here
// be dealing with either a root or a leaf. So let's do this recursively,
// as it actually is simpler from an implementation perspective anyway.
return flatMap(workspaces, (workspace) => {
return workspaces.flatMap((workspace) => {
const globbedPackageJsonPaths = glob(
resolve(packageJsonDir, workspace, "package.json"),
resolve(packageJsonDir)
);
return flatMap(globbedPackageJsonPaths, (packageJsonPath) => {
return globbedPackageJsonPaths.flatMap((packageJsonPath) => {
const packageJson = loadPackageManifest(logger, packageJsonPath);
if (packageJson === undefined) {
logger.error(`Failed to load ${packageJsonPath}`);
Expand Down