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.

Sign up for GitHub

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

Enable "strict" mode for runtime code. #3899

Merged
merged 7 commits into from
Feb 19, 2020
Merged

Enable "strict" mode for runtime code. #3899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d91eb58
Update std/manual.md
maxmellen Feb 6, 2020
d8a7858
Enable "strict" mode for runtime code.
kitsonk Dec 1, 2019
e98afdf
Update codebase to support TypeScript strict mode
maxmellen Feb 6, 2020
08c3f1f
squash! Update codebase to support TypeScript strict mode
maxmellen Feb 11, 2020
f958adf
squash! Update codebase to support TypeScript strict mode
maxmellen Feb 11, 2020
d538cf9
squash! Update codebase to support TypeScript strict mode
maxmellen Feb 18, 2020
a907271
squash! Update codebase to support TypeScript strict mode
Feb 19, 2020
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
2 changes: 1 addition & 1 deletion cli/compilers/ts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ mod tests {
.unwrap()
.code
.as_bytes()
.starts_with(b"console.log(\"Hello World\");"));
.starts_with(b"\"use strict\";\nconsole.log(\"Hello World\");"));
}

#[tokio::test]
Expand Down
1 change: 1 addition & 0 deletions cli/compilers/wasm_wrap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// @ts-nocheck
const importObject = Object.create(null);
//IMPORTS

Expand Down
6 changes: 3 additions & 3 deletions cli/js/body_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ testPerm({ net: true }, async function bodyMultipartFormData(): Promise<void> {

const formData = await body.formData();
assert(formData.has("field_1"));
assertEquals(formData.get("field_1").toString(), "value_1 \r\n");
assertEquals(formData.get("field_1")!.toString(), "value_1 \r\n");
assert(formData.has("field_2"));
});

Expand All @@ -62,7 +62,7 @@ testPerm({ net: true }, async function bodyURLEncodedFormData(): Promise<void> {

const formData = await body.formData();
assert(formData.has("field_1"));
assertEquals(formData.get("field_1").toString(), "Hi");
assertEquals(formData.get("field_1")!.toString(), "Hi");
assert(formData.has("field_2"));
assertEquals(formData.get("field_2").toString(), "<Deno>");
assertEquals(formData.get("field_2")!.toString(), "<Deno>");
});
20 changes: 17 additions & 3 deletions cli/js/buffer_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// This code has been ported almost directly from Go's src/bytes/buffer_test.go
// Copyright 2009 The Go Authors. All rights reserved. BSD license.
// https://github.com/golang/go/blob/master/LICENSE
import { assertEquals, test } from "./test_util.ts";
import { assert, assertEquals, test } from "./test_util.ts";

const { Buffer, readAll, readAllSync, writeAll, writeAllSync } = Deno;
type Buffer = Deno.Buffer;
Expand Down Expand Up @@ -78,12 +78,16 @@ function repeat(c: string, bytes: number): Uint8Array {

test(function bufferNewBuffer(): void {
init();
assert(testBytes);
assert(testString);
const buf = new Buffer(testBytes.buffer as ArrayBuffer);
check(buf, testString);
});

test(async function bufferBasicOperations(): Promise<void> {
init();
assert(testBytes);
assert(testString);
const buf = new Buffer();
for (let i = 0; i < 5; i++) {
check(buf, "");
Expand Down Expand Up @@ -134,8 +138,8 @@ test(async function bufferLargeByteWrites(): Promise<void> {
const buf = new Buffer();
const limit = 9;
for (let i = 3; i < limit; i += 3) {
const s = await fillBytes(buf, "", 5, testBytes);
await empty(buf, s, new Uint8Array(Math.floor(testString.length / i)));
const s = await fillBytes(buf, "", 5, testBytes!);
await empty(buf, s, new Uint8Array(Math.floor(testString!.length / i)));
}
check(buf, "");
});
Expand All @@ -161,6 +165,8 @@ test(async function bufferTooLargeByteWrites(): Promise<void> {

test(async function bufferLargeByteReads(): Promise<void> {
init();
assert(testBytes);
assert(testString);
const buf = new Buffer();
for (let i = 3; i < 30; i += 3) {
const n = Math.floor(testBytes.byteLength / i);
Expand All @@ -177,6 +183,8 @@ test(function bufferCapWithPreallocatedSlice(): void {

test(async function bufferReadFrom(): Promise<void> {
init();
assert(testBytes);
assert(testString);
const buf = new Buffer();
for (let i = 3; i < 30; i += 3) {
const s = await fillBytes(
Expand All @@ -194,6 +202,8 @@ test(async function bufferReadFrom(): Promise<void> {

test(async function bufferReadFromSync(): Promise<void> {
init();
assert(testBytes);
assert(testString);
const buf = new Buffer();
for (let i = 3; i < 30; i += 3) {
const s = await fillBytes(
Expand Down Expand Up @@ -236,6 +246,7 @@ test(async function bufferTestGrow(): Promise<void> {

test(async function testReadAll(): Promise<void> {
init();
assert(testBytes);
const reader = new Buffer(testBytes.buffer as ArrayBuffer);
const actualBytes = await readAll(reader);
assertEquals(testBytes.byteLength, actualBytes.byteLength);
Expand All @@ -246,6 +257,7 @@ test(async function testReadAll(): Promise<void> {

test(function testReadAllSync(): void {
init();
assert(testBytes);
const reader = new Buffer(testBytes.buffer as ArrayBuffer);
const actualBytes = readAllSync(reader);
assertEquals(testBytes.byteLength, actualBytes.byteLength);
Expand All @@ -256,6 +268,7 @@ test(function testReadAllSync(): void {

test(async function testWriteAll(): Promise<void> {
init();
assert(testBytes);
const writer = new Buffer();
await writeAll(writer, testBytes);
const actualBytes = writer.bytes();
Expand All @@ -267,6 +280,7 @@ test(async function testWriteAll(): Promise<void> {

test(function testWriteAllSync(): void {
init();
assert(testBytes);
const writer = new Buffer();
writeAllSync(writer, testBytes);
const actualBytes = writer.bytes();
Expand Down
10 changes: 9 additions & 1 deletion cli/js/chmod_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
import { testPerm, assertEquals } from "./test_util.ts";
import { testPerm, assert, assertEquals } from "./test_util.ts";

const isNotWindows = Deno.build.os !== "win";

Expand All @@ -16,6 +16,7 @@ testPerm({ read: true, write: true }, function chmodSyncSuccess(): void {
// Check success when not on windows
if (isNotWindows) {
const fileInfo = Deno.statSync(filename);
assert(fileInfo.mode);
assertEquals(fileInfo.mode & 0o777, 0o777);
}
});
Expand All @@ -35,14 +36,17 @@ if (isNotWindows) {
Deno.symlinkSync(filename, symlinkName);

let symlinkInfo = Deno.lstatSync(symlinkName);
assert(symlinkInfo.mode);
const symlinkMode = symlinkInfo.mode & 0o777; // platform dependent

Deno.chmodSync(symlinkName, 0o777);

// Change actual file mode, not symlink
const fileInfo = Deno.statSync(filename);
assert(fileInfo.mode);
assertEquals(fileInfo.mode & 0o777, 0o777);
symlinkInfo = Deno.lstatSync(symlinkName);
assert(symlinkInfo.mode);
assertEquals(symlinkInfo.mode & 0o777, symlinkMode);
}
);
Expand Down Expand Up @@ -86,6 +90,7 @@ testPerm({ read: true, write: true }, async function chmodSuccess(): Promise<
// Check success when not on windows
if (isNotWindows) {
const fileInfo = Deno.statSync(filename);
assert(fileInfo.mode);
assertEquals(fileInfo.mode & 0o777, 0o777);
}
});
Expand All @@ -105,14 +110,17 @@ if (isNotWindows) {
Deno.symlinkSync(filename, symlinkName);

let symlinkInfo = Deno.lstatSync(symlinkName);
assert(symlinkInfo.mode);
const symlinkMode = symlinkInfo.mode & 0o777; // platform dependent

await Deno.chmod(symlinkName, 0o777);

// Just change actual file mode, not symlink
const fileInfo = Deno.statSync(filename);
assert(fileInfo.mode);
assertEquals(fileInfo.mode & 0o777, 0o777);
symlinkInfo = Deno.lstatSync(symlinkName);
assert(symlinkInfo.mode);
assertEquals(symlinkInfo.mode & 0o777, symlinkMode);
}
);
Expand Down
3 changes: 1 addition & 2 deletions cli/js/compiler_host.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ export const defaultBundlerOptions: ts.CompilerOptions = {
export const defaultCompileOptions: ts.CompilerOptions = {
allowJs: true,
allowNonTsExtensions: true,
// TODO(#3324) Enable strict mode for user code.
// strict: true,
strict: true,
checkJs: false,
esModuleInterop: true,
module: ts.ModuleKind.ESNext,
Expand Down
7 changes: 5 additions & 2 deletions cli/js/console_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ const customInspect = Deno.symbols.customInspect;
const {
Console,
stringifyArgs
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} = Deno[Deno.symbols.internal] as any;
// @ts-ignore TypeScript (as of 3.7) does not support indexing namespaces by symbol
} = Deno[Deno.symbols.internal];

function stringify(...args: unknown[]): string {
return stringifyArgs(args).replace(/\n$/, "");
Expand Down Expand Up @@ -306,6 +306,7 @@ test(function consoleTestCallToStringOnLabel(): void {
for (const method of methods) {
let hasCalled = false;

// @ts-ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

There should be a way to type it so we don't need the ignore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similarly to #3899 (comment), the time of the argument passed to console[method] still wouldn't be correct so I'm not sure how to improve on this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you would have to do something like to remove the @ts-ignore:

const methods: Array<keyof Console> = ["count", "countReset", "time", "timeLog", "timeEnd"];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I type methods as Array<keyof typeof Console> (apparently Console refers to a value in this context) I then get the following error:

error TS7053: Element implicitly has an 'any' type because expression of type 'string | number | symbol' can't be used to index type 'Console'.
  No index signature with a parameter of type 'string' was found on type 'Console'.

► file:///Users/max/Developer/deno/cli/js/console_test.ts:310:5

310     console[method]({
        ~~~~~~~~~~~~~~~

And even if we got over that error, then we'd have the issue that { toString(): void { hasCalled = true; } } wouldn't be considered a valid argument to these functions anyway (for instance, count only accepts a string), so I think the best thing to do is switch methods back to implicitly be of type string[] as before my changes and keep the @ts-ignore.

Copy link
Contributor

Choose a reason for hiding this comment

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

🤷‍♂ ok! you win! :-D

console[method]({
toString(): void {
hasCalled = true;
Expand Down Expand Up @@ -451,6 +452,7 @@ test(function consoleGroup(): void {
// console.group with console.warn test
test(function consoleGroupWarn(): void {
mockConsole((console, _out, _err, both): void => {
assert(both);
console.warn("1");
console.group();
console.warn("2");
Expand Down Expand Up @@ -694,6 +696,7 @@ test(function consoleDirXml(): void {
test(function consoleTrace(): void {
mockConsole((console, _out, err): void => {
console.trace("%s", "custom message");
assert(err);
assert(err.toString().includes("Trace: custom message"));
});
});
4 changes: 2 additions & 2 deletions cli/js/error_stack_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
import { test, assert } from "./test_util.ts";

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const { setPrepareStackTrace } = Deno[Deno.symbols.internal] as any;
// @ts-ignore TypeScript (as of 3.7) does not support indexing namespaces by symbol
const { setPrepareStackTrace } = Deno[Deno.symbols.internal];

interface CallSite {
getThis(): unknown;
Expand Down
22 changes: 18 additions & 4 deletions cli/js/event_target_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ import { test, assertEquals } from "./test_util.ts";
test(function addEventListenerTest(): void {
const document = new EventTarget();

// @ts-ignore tests ignoring the type system for resilience
assertEquals(document.addEventListener("x", null, false), undefined);
// @ts-ignore
assertEquals(document.addEventListener("x", null, true), undefined);
// @ts-ignore
assertEquals(document.addEventListener("x", null), undefined);
});

Expand All @@ -14,7 +17,7 @@ test(function constructedEventTargetCanBeUsedAsExpected(): void {
const event = new Event("foo", { bubbles: true, cancelable: false });
let callCount = 0;

const listener = (e): void => {
const listener = (e: Event): void => {
assertEquals(e, event);
++callCount;
};
Expand All @@ -34,11 +37,19 @@ test(function constructedEventTargetCanBeUsedAsExpected(): void {

test(function anEventTargetCanBeSubclassed(): void {
class NicerEventTarget extends EventTarget {
on(type, callback?, options?): void {
on(
type: string,
callback: (e: Event) => void | null,
options?: __domTypes.AddEventListenerOptions
): void {
this.addEventListener(type, callback, options);
}

off(type, callback?, options?): void {
off(
type: string,
callback: (e: Event) => void | null,
options?: __domTypes.EventListenerOptions
): void {
this.removeEventListener(type, callback, options);
}
}
Expand All @@ -60,8 +71,11 @@ test(function anEventTargetCanBeSubclassed(): void {

test(function removingNullEventListenerShouldSucceed(): void {
const document = new EventTarget();
// @ts-ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

again, what are we ignoring and why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same issue as #3899 (comment)

assertEquals(document.removeEventListener("x", null, false), undefined);
// @ts-ignore
assertEquals(document.removeEventListener("x", null, true), undefined);
// @ts-ignore
assertEquals(document.removeEventListener("x", null), undefined);
});

Expand All @@ -70,7 +84,7 @@ test(function constructedEventTargetUseObjectPrototype(): void {
const event = new Event("toString", { bubbles: true, cancelable: false });
let callCount = 0;

const listener = (e): void => {
const listener = (e: Event): void => {
assertEquals(e, event);
++callCount;
};
Expand Down
13 changes: 7 additions & 6 deletions cli/js/event_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
import { test, assertEquals, assertNotEquals } from "./test_util.ts";
import { test, assertEquals, assert } from "./test_util.ts";

test(function eventInitializedWithType(): void {
const type = "click";
Expand Down Expand Up @@ -70,7 +70,8 @@ test(function eventPreventDefaultSuccess(): void {
});

test(function eventInitializedWithNonStringType(): void {
const type = undefined;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const type: any = undefined;
const event = new Event(type);

assertEquals(event.isTrusted, false);
Expand All @@ -84,12 +85,12 @@ test(function eventInitializedWithNonStringType(): void {
// ref https://github.com/web-platform-tests/wpt/blob/master/dom/events/Event-isTrusted.any.js
test(function eventIsTrusted(): void {
const desc1 = Object.getOwnPropertyDescriptor(new Event("x"), "isTrusted");
assertNotEquals(desc1, undefined);
assert(desc1);
assertEquals(typeof desc1.get, "function");

const desc2 = Object.getOwnPropertyDescriptor(new Event("x"), "isTrusted");
assertNotEquals(desc2, undefined);
assertEquals(typeof desc2.get, "function");
assert(desc2);
assertEquals(typeof desc2!.get, "function");

assertEquals(desc1.get, desc2.get);
assertEquals(desc1!.get, desc2!.get);
});
Loading