set strictNullChecks to true in tsconfig.json and fix errors #422

oltolm · 2024-04-06T11:09:29Z

Another cleanup. I set strictNullChecks to true in tsconfig.json and the fixed all the errors.

codecov-commenter · 2024-04-06T11:10:07Z

Codecov Report

Attention: Patch coverage is 2.91262% with 100 lines in your changes are missing coverage. Please review.

Project coverage is 20.29%. Comparing base (1b9f13e) to head (417a462).

Files	Patch %	Lines
src/frontend/extension.ts	0.00%	26 Missing ⚠️
src/backend/mi2/mi2.ts	0.00%	24 Missing ⚠️
src/mibase.ts	0.00%	22 Missing ⚠️
src/gdb.ts	0.00%	5 Missing ⚠️
src/lldb.ts	0.00%	5 Missing ⚠️
src/mago.ts	0.00%	5 Missing ⚠️
src/backend/linux/console.ts	0.00%	4 Missing ⚠️
src/backend/mi2/mi2lldb.ts	0.00%	4 Missing ⚠️
src/backend/mi2/mi2mago.ts	0.00%	2 Missing ⚠️
src/backend/mi_parse.ts	50.00%	0 Missing and 2 partials ⚠️
... and 1 more

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #422      +/-   ##
==========================================
- Coverage   20.47%   20.29%   -0.18%     
==========================================
  Files          14       14              
  Lines        1807     1818      +11     
  Branches      392      404      +12     
==========================================
- Hits          370      369       -1     
- Misses       1392     1402      +10     
- Partials       45       47       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

WebFreak001 · 2024-04-07T20:28:38Z

src/backend/mi2/mi2.ts

@@ -589,9 +589,9 @@ export class MI2 extends EventEmitter implements IBackend {
 			let location = "";
 			if (breakpoint.countCondition) {
 				if (breakpoint.countCondition[0] == ">")
-					location += "-i " + numRegex.exec(breakpoint.countCondition.substring(1))[0] + " ";
+					location += "-i " + numRegex.exec(breakpoint.countCondition.substring(1))?.[0] ?? "" + " ";


we should move the numRegex.exec(...) into a separate variable, then check that it matches, then either it must match and otherwise print an error and do nothing, or fall back to some value like 0

Since we are using -i, an argument after it is required and we can't just leave it blank like that. (haven't checked GDB documentation to actually verify this statement though, it just looks like it would work that way)

WebFreak001 · 2024-04-07T20:31:39Z

src/backend/mi2/mi2lldb.ts

@@ -10,7 +9,7 @@ export class MI2_LLDB extends MI2 {
 		// Since the CWD is expected to be an absolute path in the debugger's environment, we can test
 		// that to determine the path type used by the debugger and use the result of that test to
 		// select the correct API to check whether the target path is an absolute path.
-		const debuggerPath = path.posix.isAbsolute(cwd) ? path.posix : path.win32;
+		const debuggerPath = posix.isAbsolute(cwd) ? posix : win32;


this looks very weird, I don't think the import separation gains us anything here. In node.js path used to be a library that you also always just imported as a whole, although I haven't kept up with the JS world there what is used now.

But considering that we are assigning it to debuggerPath, so it can be used like debuggerPath.join(...) instead of path.join(...) to get the correct path for the current platform, I'd much prefer keeping the path. prefix

I reverted all changes to imports.

WebFreak001 · 2024-04-07T20:35:15Z

src/frontend/extension.ts

-	context.subscriptions.push(vscode.commands.registerCommand("code-debug.getFileNameNoExt", () => {
-		if (!vscode.window.activeTextEditor || !vscode.window.activeTextEditor.document || !vscode.window.activeTextEditor.document.fileName) {
-			vscode.window.showErrorMessage("No editor with valid file name active");
+export function activate(context: ExtensionContext) {


keep the vscode import, it's the recommended style from the vscode docs and also makes finding symbols much easier than global imports. In fact I think all of these imports that you changed are much better in a namespace and shouldn't be unwrapped like this. Especially generic stuff like join or connect doesn't make much sense to import on its own and just makes the caller code less readable.

WebFreak001 · 2024-04-07T20:36:59Z

src/gdb.ts

@@ -68,8 +69,8 @@ class GDBDebugSession extends MI2DebugSession {
 		this.started = false;
 		this.crashed = false;
 		this.setValuesFormattingMode(args.valuesFormatting);
-		this.miDebugger.printCalls = !!args.printCalls;
-		this.miDebugger.debugOutput = !!args.showDevDebugOutput;
+		this.miDebugger.printCalls = args.printCalls;


args is user supplied and may be literally any JSON type. (even though it will show type errors to the user in vscode) Keep the boolean cast to make sure we don't break any type constraints in typescript from here.

WebFreak001 · 2024-04-07T20:37:43Z

src/gdb.ts

-		this.miDebugger.printCalls = !!args.printCalls;
-		this.miDebugger.debugOutput = !!args.showDevDebugOutput;
+		this.miDebugger.printCalls = args.printCalls;
+		this.miDebugger.debugOutput = args.showDevDebugOutput;


WebFreak001 · 2024-04-07T20:37:46Z

src/lldb.ts

-		this.miDebugger.printCalls = !!args.printCalls;
-		this.miDebugger.debugOutput = !!args.showDevDebugOutput;
+		this.miDebugger.printCalls = args.printCalls;
+		this.miDebugger.debugOutput = args.showDevDebugOutput;


WebFreak001 · 2024-04-07T20:37:49Z

src/lldb.ts

-		this.miDebugger.printCalls = !!args.printCalls;
-		this.miDebugger.debugOutput = !!args.showDevDebugOutput;
+		this.miDebugger.printCalls = args.printCalls;
+		this.miDebugger.debugOutput = args.showDevDebugOutput;


WebFreak001 · 2024-04-07T20:37:54Z

src/mago.ts

-		this.miDebugger.printCalls = !!args.printCalls;
-		this.miDebugger.debugOutput = !!args.showDevDebugOutput;
+		this.miDebugger.printCalls = args.printCalls;
+		this.miDebugger.debugOutput = args.showDevDebugOutput;


WebFreak001 · 2024-04-07T20:37:57Z

src/mago.ts

-		this.miDebugger.printCalls = !!args.printCalls;
-		this.miDebugger.debugOutput = !!args.showDevDebugOutput;
+		this.miDebugger.printCalls = args.printCalls;
+		this.miDebugger.debugOutput = args.showDevDebugOutput;


WebFreak001 · 2024-04-07T20:39:04Z

src/mibase.ts

-			if (!fs.existsSync(systemPath.join(os.tmpdir(), "code-debug-sockets")))
-				fs.mkdirSync(systemPath.join(os.tmpdir(), "code-debug-sockets"));
-			this.commandServer.listen(this.serverPath = systemPath.join(os.tmpdir(), "code-debug-sockets", ("Debug-Instance-" + Math.floor(Math.random() * 36 * 36 * 36 * 36).toString(36)).toLowerCase()));
+			if (!existsSync(join(tmpdir(), "code-debug-sockets")))


keep systemPath, it's written explicitly to differentiate that this is path using separators using the host OS config, rather than the debuggerPath, which uses separates on the platform that the debugger is running on

WebFreak001 · 2024-04-07T20:39:39Z

src/mibase.ts

@@ -188,7 +188,7 @@ export class MI2DebugSession extends DebugSession {
 		else
 			this.miDebugger.stop();
 		this.commandServer.close();
-		this.commandServer = undefined;
+		// this.commandServer = undefined;


remove line

WebFreak001 · 2024-04-07T20:40:37Z

src/mibase.ts

+				variablesReference: 0,
+				result: ""
+			};
+			this.sendResponse(response);


sendErrorResponse instead of plain sendResponse

oltolm · 2024-04-27T11:12:35Z

I fixed everything.

oltolm · 2024-05-30T09:35:42Z

Is there still something to do to get it accepted?

WebFreak001 reviewed Apr 7, 2024

View reviewed changes

WebFreak001 requested changes Apr 7, 2024

View reviewed changes

set strictNullChecks to true in tsconfig.json and fix errors

b2100e7

oltolm force-pushed the strictnullchecks branch from 417a462 to b2100e7 Compare April 16, 2024 22:35

oltolm closed this Aug 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

set strictNullChecks to true in tsconfig.json and fix errors #422

set strictNullChecks to true in tsconfig.json and fix errors #422

oltolm commented Apr 6, 2024

codecov-commenter commented Apr 6, 2024 •

edited

Loading

WebFreak001 Apr 7, 2024

oltolm Apr 16, 2024

WebFreak001 Apr 7, 2024

oltolm Apr 16, 2024

WebFreak001 Apr 7, 2024

WebFreak001 Apr 7, 2024

WebFreak001 Apr 7, 2024

WebFreak001 Apr 7, 2024

WebFreak001 Apr 7, 2024

WebFreak001 Apr 7, 2024

WebFreak001 Apr 7, 2024

WebFreak001 Apr 7, 2024

WebFreak001 Apr 7, 2024

oltolm Apr 16, 2024

WebFreak001 Apr 7, 2024

oltolm Apr 16, 2024

oltolm commented Apr 27, 2024

oltolm commented May 30, 2024

set strictNullChecks to true in tsconfig.json and fix errors #422

set strictNullChecks to true in tsconfig.json and fix errors #422

Conversation

oltolm commented Apr 6, 2024

codecov-commenter commented Apr 6, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oltolm commented Apr 27, 2024

oltolm commented May 30, 2024

codecov-commenter commented Apr 6, 2024 •

edited

Loading