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

Strict null checks! #845

Merged
merged 30 commits into from
Oct 20, 2018
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
cc401fa
Turn strictNullChecks on
Gerrit0 Aug 25, 2018
752d8cf
Remove yarn error log, add to .gitignore
Gerrit0 Aug 25, 2018
93958a5
Resolve most unit test issues
Gerrit0 Aug 26, 2018
8f90d12
Restore name splitting behavior
Gerrit0 Aug 27, 2018
1735d4d
Fix missing implemented interfaces
Gerrit0 Aug 27, 2018
820077a
#837 Add contributing.md (#841)
Gerrit0 Sep 3, 2018
c109e8b
Add localization plugin to Readme.md plugin section (#850)
zdrawku Sep 3, 2018
589f15a
Use a dummy readme when testing the renderer (#857)
Towerism Sep 19, 2018
eeccb6b
Peer review comments
Gerrit0 Sep 26, 2018
f7d5683
fix(HelperStack): deactivate theme resources correctly (#865)
Aleksandyr Oct 3, 2018
f0166cf
Upgrade typescript to v3.1.x (#863)
ahocevar Oct 4, 2018
1108878
fix: No error is thrown when adding plug-ins repeatedly(#847)
Houfeng Oct 4, 2018
b371fd4
0.13.0-0
aciccarello Oct 4, 2018
7247605
0.13.0
aciccarello Oct 10, 2018
ee162c9
simplify event test types
aciccarello Oct 12, 2018
daab024
simplify types
aciccarello Oct 12, 2018
5759cb5
correct getter in EventDispatcher
aciccarello Oct 12, 2018
20c90c7
Turn strictNullChecks on
Gerrit0 Aug 25, 2018
179f5af
Remove yarn error log, add to .gitignore
Gerrit0 Aug 25, 2018
9bd8836
Resolve most unit test issues
Gerrit0 Aug 26, 2018
e1bb10c
Restore name splitting behavior
Gerrit0 Aug 27, 2018
fbe3a5a
Fix missing implemented interfaces
Gerrit0 Aug 27, 2018
fc192ce
Peer review comments
Gerrit0 Sep 26, 2018
e110441
simplify event test types
aciccarello Oct 12, 2018
4dcf9b1
simplify types
aciccarello Oct 12, 2018
4bf87f6
correct getter in EventDispatcher
aciccarello Oct 12, 2018
bc469d8
remove uncecessary type assertion
aciccarello Oct 12, 2018
cd840fb
Merge branch 'strict-null-checks' of https://github.com/Gerrit0/typed…
aciccarello Oct 12, 2018
5517705
Remove readonly on option properties
Gerrit0 Oct 18, 2018
4d4ff46
Pin highlight.js version to 9.12.0
Gerrit0 Oct 19, 2018
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
.baseDir.js
.baseDir.ts
yarn.lock
yarn-error.log

/src/typings/typescript/typescript.js

Expand Down
8 changes: 5 additions & 3 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,19 @@
{
"type": "node",
"request": "launch",
"name": "Launch Program",
"name": "Debug Tests",
"program": "${workspaceRoot}/node_modules/mocha/bin/_mocha",
"cwd": "${workspaceRoot}",
"args": [
"--no-timeouts"
"--no-timeouts",
"dist/test/**/*.js"
],
"outFiles": [
"${workspaceRoot}/lib/**/*.js",
"${workspaceRoot}/test/**/*.js"
],
"preLaunchTask": "build",
"sourceMaps": true
}
]
}
}
32 changes: 22 additions & 10 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,23 @@
{
// See https://go.microsoft.com/fwlink/?LinkId=733558
// for the documentation about the tasks.json format
"version": "0.1.0",
"command": "tsc",
"isShellCommand": true,
"args": ["-w", "-p", "."],
"showOutput": "silent",
"isWatching": true,
"problemMatcher": "$tsc-watch"
}
// See https://go.microsoft.com/fwlink/?LinkId=733558
// for the documentation about the tasks.json format
"version": "2.0.0",
"tasks": [
{
"identifier": "build",
"type": "grunt",
"task": "default",
"problemMatcher": [
"$tsc"
]
},
{
"identifier": "build_and_test",
"type": "grunt",
"task": "build_and_test",
"problemMatcher": [
"$tsc"
]
}
]
}
20 changes: 10 additions & 10 deletions src/lib/application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { Serializer } from './serialization';
import { ProjectReflection } from './models/index';
import { Logger, ConsoleLogger, CallbackLogger, PluginHost, writeFile } from './utils/index';

import { AbstractComponent, ChildableComponent, Component, Option } from './utils/component';
import { AbstractComponent, ChildableComponent, Component, Option, DUMMY_APPLICATION_OWNER } from './utils/component';
import { Options, OptionsReadMode, OptionsReadResult } from './utils/options/index';
import { ParameterType } from './utils/options/declaration';

Expand Down Expand Up @@ -67,21 +67,21 @@ export class Application extends ChildableComponent<Application, AbstractCompone
defaultValue: 'console',
type: ParameterType.Mixed
})
loggerType: string|Function;
readonly loggerType!: string|Function;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these marked readonly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe my thought process was that they should only be set by option providers, not manually, it would be fine to remove the readonly.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should remove readonly to avoid unnecessary changes for anyone who may be touching the javascript apis.


@Option({
name: 'ignoreCompilerErrors',
help: 'Should TypeDoc generate documentation pages even after the compiler has returned errors?',
type: ParameterType.Boolean
})
ignoreCompilerErrors: boolean;
readonly ignoreCompilerErrors!: boolean;

@Option({
name: 'exclude',
help: 'Define patterns for excluded files when specifying paths.',
type: ParameterType.Array
})
exclude: Array<string>;
readonly exclude!: Array<string>;

/**
* The version number of TypeDoc.
Expand All @@ -94,7 +94,7 @@ export class Application extends ChildableComponent<Application, AbstractCompone
* @param options An object containing the options that should be used.
*/
constructor(options?: Object) {
super(null);
super(DUMMY_APPLICATION_OWNER);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why wouldn't we want the owner to be nullable?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Searching for .owner finds 161 uses, none of which handle nullable owners. In interest of avoiding more changes, I decided to leave it non-nullable.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would setting the owner to itself be an alternative to using a symbol and needing to complicate the type?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Typescript allowed super(this), it would be. However since this isn't allowed, we either need to do super(undefined as any) and add the owner manually, or use a symbol.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'd like to double back on this, possibly at a later time to find a cleaner solution. For now it works.


this.logger = new ConsoleLogger();
this.converter = this.addComponent<Converter>('converter', Converter);
Expand Down Expand Up @@ -153,9 +153,9 @@ export class Application extends ChildableComponent<Application, AbstractCompone
* Run the converter for the given set of files and return the generated reflections.
*
* @param src A list of source that should be compiled and converted.
* @returns An instance of ProjectReflection on success, NULL otherwise.
* @returns An instance of ProjectReflection on success, undefined otherwise.
*/
public convert(src: string[]): ProjectReflection {
public convert(src: string[]): ProjectReflection | undefined {
this.logger.writeln('Using TypeScript %s from %s', this.getTypeScriptVersion(), this.getTypeScriptPath());

const result = this.converter.convert(src);
Expand All @@ -165,7 +165,7 @@ export class Application extends ChildableComponent<Application, AbstractCompone
this.logger.resetErrors();
return result.project;
} else {
return null;
return;
}
} else {
return result.project;
Expand All @@ -188,7 +188,7 @@ export class Application extends ChildableComponent<Application, AbstractCompone
* @param out The path the documentation should be written to.
* @returns TRUE if the documentation could be generated successfully, otherwise FALSE.
*/
public generateDocs(input: any, out: string): boolean {
public generateDocs(input: ProjectReflection | string[], out: string): boolean {
const project = input instanceof ProjectReflection ? input : this.convert(input);
if (!project) {
return false;
Expand Down Expand Up @@ -246,7 +246,7 @@ export class Application extends ChildableComponent<Application, AbstractCompone
* @param inputFiles The list of files that should be expanded.
* @returns The list of input files with expanded directories.
*/
public expandInputFiles(inputFiles?: string[]): string[] {
public expandInputFiles(inputFiles: string[] = []): string[] {
let files: string[] = [];
const exclude: Array<IMinimatch> = this.exclude ? this.exclude.map(pattern => new Minimatch(pattern, {dot: true})) : [];

Expand Down
11 changes: 5 additions & 6 deletions src/lib/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,39 +20,38 @@ export class CliApplication extends Application {
help: 'Specifies the location the documentation should be written to.',
hint: ParameterHint.Directory
})
out: string;
out!: string;

@Option({
name: 'json',
help: 'Specifies the location and file name a json file describing the project is written to.',
hint: ParameterHint.File
})
json: string;
json!: string;

@Option({
name: 'version',
short: 'v',
help: 'Print the TypeDoc\'s version.',
type: ParameterType.Boolean
})
version: boolean;
version!: boolean;

@Option({
name: 'help',
short: 'h',
help: 'Print this message.',
type: ParameterType.Boolean
})
help: boolean;
help!: boolean;

/**
* Run TypeDoc from the command line.
*/
protected bootstrap(options?: Object): OptionsReadResult {
const result = super.bootstrap(options);
if (result.hasErrors) {
process.exit(ExitCode.OptionError);
return;
return process.exit(ExitCode.OptionError);
}

if (this.version) {
Expand Down
10 changes: 5 additions & 5 deletions src/lib/converter/components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ export abstract class ConverterNodeComponent<T extends ts.Node> extends Converte
/**
* List of supported TypeScript syntax kinds.
*/
supports: ts.SyntaxKind[];
abstract supports: ts.SyntaxKind[];

abstract convert(context: Context, node: T): Reflection;
abstract convert(context: Context, node: T): Reflection | undefined;
}

export abstract class ConverterTypeComponent extends ConverterComponent {
Expand All @@ -39,17 +39,17 @@ export interface TypeTypeConverter<T extends ts.Type> extends ConverterTypeCompo
/**
* Convert the given type to its type reflection.
*/
convertType(context: Context, type: T): Type;
convertType(context: Context, type: T): Type | undefined;
}

export interface TypeNodeConverter<T extends ts.Type, N extends ts.Node> extends ConverterTypeComponent {
/**
* Test whether this converter can handle the given TypeScript node.
*/
supportsNode(context: Context, node: N, type: T): boolean;
supportsNode(context: Context, node: N, type: T | undefined): boolean;

/**
* Convert the given type node to its type reflection.
*/
convertNode(context: Context, node: N, type: T): Type;
convertNode(context: Context, node: N, type: T | undefined): Type | undefined;
}
Loading