Skip to content

Enabled strict config - #515

Merged
Perryvw merged 22 commits into
masterfrom
strict
Apr 14, 2019
Merged

Enabled strict config#515
Perryvw merged 22 commits into
masterfrom
strict

Conversation

@Perryvw

@Perryvw Perryvw commented Apr 9, 2019

Copy link
Copy Markdown
Member

WIP: Still need all strict issues to be solved.

Also made some preliminary interface changes to the transformer (although that should probably be finalized in a separate PR).

Comment thread src/LuaPrinter.ts Outdated
Comment thread src/LuaTransformer.ts Outdated
Comment thread src/LuaTransformer.ts Outdated
Comment thread src/LuaTransformer.ts Outdated
Comment thread src/TSHelper.ts Outdated
Comment thread src/TSHelper.ts Outdated
Comment thread src/TSHelper.ts Outdated
@Perryvw
Perryvw marked this pull request as ready for review April 13, 2019 14:31
@Perryvw
Perryvw requested a review from tomblind April 13, 2019 14:31
Comment thread src/LuaAST.ts Outdated
Comment thread src/LuaTransformer.ts Outdated
Comment thread src/LuaTransformer.ts Outdated
Comment thread src/LuaTransformer.ts Outdated
Comment thread src/LuaTransformer.ts Outdated
Comment thread src/LuaTransformer.ts Outdated
Comment thread jest.config.js Outdated
"ts-jest": {
tsConfig: "<rootDir>/test/tsconfig.json",
diagnostics: { warnOnly: !isCI },
diagnostics: { warnOnly: false },

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have you tried setting "strict": true in test/tsconfig.json instead?

Comment thread src/TSTLErrors.ts Outdated
new TranspileError(`Unsupported property on ${parentName}: ${property}`, node);

public static UnsupportedForTarget = (functionality: string, version: string, node: ts.Node) =>
public static UnsupportedForTarget = (functionality: string, version: string | undefined, node: ts.Node) =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lua undefined doesn't sound right, won't it be better to handle it in the caller? It won't complain about nullable value if it would be this.luaTarget = this.options.luaTarget || LuaTarget.LuaJIT in the LuaTransformer constructor. That also would avoid mutating passed options object.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good idea

Comment thread src/LuaTransformer.ts Outdated
@@ -4547,11 +4748,12 @@ export class LuaTransformer {
throw TSTLErrors.ReferencedBeforeDeclaration(identifier);
}

} else {
} else if (symbolId !== undefined) {
const sid = symbolId; // Capture non-undefined variable

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It might be simpler to replace forEach loop with a for..of instead

@Perryvw
Perryvw merged commit 27ae2a8 into master Apr 14, 2019
@Perryvw
Perryvw deleted the strict branch April 14, 2019 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants