Skip to content

Commit

Permalink
Add support for filenames in intentionallyNotExported
Browse files Browse the repository at this point in the history
Resolves #1715
  • Loading branch information
Gerrit0 committed Oct 2, 2021
1 parent 9f9bc38 commit 042680c
Show file tree
Hide file tree
Showing 4 changed files with 146 additions and 37 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@

- TypeDoc will now recognize `@param` comments for destructured parameters and rename `__namedParameters` to the name specified
in the `@param` comment if the number of `@param` comments match the number of parameters, resolves #1703.
- The `intentionallyNotExported` option may now include file names/paths to limit its scope, for example, the following
will suppress warnings from `Foo` in `src/foo.ts` not being exported, but will not suppress warnings if another `Foo`
declared in `src/utils/foo.ts` is not exported.
```json
{
"intentionallyNotExported": ["src/foo.ts:Foo"]
}
```

### Bug Fixes

Expand Down
117 changes: 83 additions & 34 deletions src/lib/validation/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,52 +12,104 @@ import {
} from "../models";
import type { Logger } from "../utils";

function makeIntentionallyExportedHelper(
intentional: readonly string[],
logger: Logger
) {
const used = new Set<number>();
const processed: [string, string][] = intentional.map((v) => {
const index = v.lastIndexOf(":");
if (index === -1) {
return ["", v];
}
return [v.substring(0, index), v.substring(index + 1)];
});

return {
has(symbol: ts.Symbol) {
// If it isn't declared anywhere, we can't produce a good error message about where
// the non-exported symbol is, so even if it isn't ignored, pretend it is. In practice,
// this will happen incredibly rarely, since symbols without declarations are very rare.
// I know of only two instances:
// 1. `undefined` in `globalThis`
// 2. Properties on non-homomorphic mapped types, e.g. the symbol for "foo" on `Record<"foo", 1>`
// There might be others, so still check this here rather than asserting, but print a debug log
// so that we can possibly improve this in the future.
if (!symbol.declarations?.length) {
logger.verbose(
`The symbol ${symbol.name} has no declarations, implicitly allowing missing export.`
);
return true;
}

// Don't produce warnings for third-party symbols.
if (
symbol.declarations.some((d) =>
d.getSourceFile().fileName.includes("node_modules")
)
) {
return true;
}

for (const [index, [file, name]] of processed.entries()) {
if (
symbol.name === name &&
symbol.declarations.some((d) =>
d.getSourceFile().fileName.endsWith(file)
)
) {
used.add(index);
return true;
}
}

return false;
},
getUnused() {
return intentional.filter((_, i) => !used.has(i));
},
};
}

export function validateExports(
project: ProjectReflection,
logger: Logger,
intentionallyNotExported: readonly string[]
) {
let current: Reflection | undefined = project;
const queue: Reflection[] = [];
const intentional = new Set(intentionallyNotExported);
const seenIntentional = new Set<string>();
const intentional = makeIntentionallyExportedHelper(
intentionallyNotExported,
logger
);
const warned = new Set<ts.Symbol>();

const visitor = makeRecursiveVisitor({
reference(type) {
// If we don't have a symbol, then this was an intentionally broken reference.
const symbol = type.getSymbol();
if (!type.reflection && symbol) {
if (intentional.has(symbol.name)) {
seenIntentional.add(symbol.name);
}

if (
(symbol.flags & ts.SymbolFlags.TypeParameter) === 0 &&
!intentional.has(symbol.name) &&
!warned.has(symbol) &&
!symbol.declarations?.some((d) =>
d.getSourceFile().fileName.includes("node_modules")
)
!intentional.has(symbol) &&
!warned.has(symbol)
) {
warned.add(symbol);
const decl = symbol.declarations?.[0];
if (decl) {
const { line } = ts.getLineAndCharacterOfPosition(
decl.getSourceFile(),
decl.getStart()
);
const file = relative(
process.cwd(),
decl.getSourceFile().fileName
);

logger.warn(
`${
type.name
}, defined at ${file}:${line}, is referenced by ${current!.getFullName()} but not included in the documentation.`
);
}
const decl = symbol.declarations![0];
const { line } = ts.getLineAndCharacterOfPosition(
decl.getSourceFile(),
decl.getStart()
);
const file = relative(
process.cwd(),
decl.getSourceFile().fileName
);

logger.warn(
`${
type.name
}, defined at ${file}:${line}, is referenced by ${current!.getFullName()} but not included in the documentation.`
);
}
}
},
Expand Down Expand Up @@ -116,14 +168,11 @@ export function validateExports(
}
} while ((current = queue.shift()));

for (const x of seenIntentional) {
intentional.delete(x);
}

if (intentional.size) {
const unusedIntentional = intentional.getUnused();
if (unusedIntentional.length) {
logger.warn(
"The following symbols were marked as intentionally not exported, but were either not referenced in the documentation, or were exported:\n\t" +
Array.from(intentional).join("\n\t")
unusedIntentional.join("\n\t")
);
}
}
2 changes: 2 additions & 0 deletions src/test/converter2/validation/externalType.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import { Program } from "typescript";
export declare const program: Program;
56 changes: 53 additions & 3 deletions src/test/validation.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { equal, ok } from "assert";
import { equal, fail, ok } from "assert";
import { join, relative } from "path";
import { Logger, LogLevel } from "..";
import { validateExports } from "../lib/validation/exports";
Expand All @@ -7,7 +7,8 @@ import { getConverter2App, getConverter2Program } from "./programs";
function expectWarning(
typeName: string,
file: string,
referencingName: string
referencingName: string,
intentionallyNotExported: readonly string[] = []
) {
const app = getConverter2App();
const program = getConverter2Program();
Expand Down Expand Up @@ -52,10 +53,45 @@ function expectWarning(
}
}

validateExports(project, new LoggerCheck(), []);
validateExports(project, new LoggerCheck(), intentionallyNotExported);
ok(sawWarning, `Expected warning message for ${typeName} to be reported.`);
}

function expectNoWarning(
file: string,
intentionallyNotExported: readonly string[] = []
) {
const app = getConverter2App();
const program = getConverter2Program();
const sourceFile = program.getSourceFile(
join(__dirname, "converter2/validation", file)
);

ok(sourceFile, "Specified source file does not exist.");

const project = app.converter.convert([
{
displayName: "validation",
program,
sourceFile,
},
]);

const regex =
/(.*?), defined at (.*?):\d+, is referenced by (.*?) but not included in the documentation\./;

class LoggerCheck extends Logger {
override log(message: string, level: LogLevel) {
const match = message.match(regex);
if (level === LogLevel.Warn && match) {
fail("Expected no warnings about missing exports");
}
}
}

validateExports(project, new LoggerCheck(), intentionallyNotExported);
}

describe("validateExports", () => {
it("Should warn if a variable type is missing", () => {
expectWarning("Foo", "variable.ts", "foo");
Expand Down Expand Up @@ -93,6 +129,20 @@ describe("validateExports", () => {
expectWarning("Bar", "return.ts", "foo.foo");
});

it("Should allow filtering warnings by file name", () => {
expectNoWarning("variable.ts", ["variable.ts:Foo"]);
expectNoWarning("variable.ts", ["validation/variable.ts:Foo"]);
expectNoWarning("variable.ts", ["Foo"]);
});

it("Should not apply warnings filtered by file name to other files", () => {
expectWarning("Foo", "variable.ts", "foo", ["notVariable.ts:Foo"]);
});

it("Should not produce warnings for types originating in node_modules", () => {
expectNoWarning("externalType.ts");
});

it("Should warn if intentionallyNotExported contains unused values", () => {
const app = getConverter2App();
const program = getConverter2Program();
Expand Down

0 comments on commit 042680c

Please sign in to comment.