From 4a0b645595eaf58988c4ba951208f2d25ddd48c4 Mon Sep 17 00:00:00 2001 From: Nishant Mittal Date: Thu, 19 Sep 2024 02:45:29 +0530 Subject: [PATCH] feat(dev): introduce strict null checks (#99) * feat(dev): introduce strict null checks * feat(dev): introduce strict null checks * fix: comply with strict-null-checks --- src/index.ts | 8 ++--- src/legacyTemplates.ts | 4 +-- src/parser.ts | 16 ++++++--- src/utils/dateAndTime.ts | 6 ++-- src/utils/typescript.ts | 6 ++++ src/variables/parser.ts | 4 +++ src/variables/types/base.ts | 2 +- src/variables/types/enum.ts | 2 +- tests/parser.spec.ts | 63 +++++++++++++++++++++++++++++------ tests/utils/templates.spec.ts | 3 +- tsconfig.json | 1 + 11 files changed, 89 insertions(+), 26 deletions(-) create mode 100644 src/utils/typescript.ts diff --git a/src/index.ts b/src/index.ts index 995ef80..14019ac 100644 --- a/src/index.ts +++ b/src/index.ts @@ -98,12 +98,12 @@ joplin.plugins.register({ } const getTemplateAndPerformAction = async (action: TemplateAction) => { - const template: Note = JSON.parse(await getUserTemplateSelection()); + const template: Note = JSON.parse(await getUserTemplateSelection() || "null"); await performActionWithParsedTemplate(action, template); } const getNotebookDefaultTemplatesDisplayData = async (settings: DefaultTemplatesConfigSetting): Promise => { - const getDisplayDataForNotebook = async (notebookId: string, defaultTemplateNoteId: string, defaultTemplateTodoId: string): Promise => { + const getDisplayDataForNotebook = async (notebookId: string, defaultTemplateNoteId: string | null, defaultTemplateTodoId: string | null): Promise => { const promiseGroup = new PromiseGroup(); promiseGroup.set("notebook", getFolderFromId(notebookId)); promiseGroup.set("noteTemplate", getTemplateFromId(defaultTemplateNoteId)); @@ -196,7 +196,7 @@ joplin.plugins.register({ name: "setDefaultTemplateForNotebook", label: "Set default template for notebook", execute: async () => { - const folder: Folder | null = JSON.parse(await getUserFolderSelection()); + const folder: Folder | null = JSON.parse(await getUserFolderSelection() || "null"); if (folder === null) return; const templateId = await getUserTemplateSelection("id", `Default template for "${folder.title}":`); @@ -214,7 +214,7 @@ joplin.plugins.register({ name: "clearDefaultTemplatesForNotebook", label: "Clear default templates for notebook", execute: async () => { - const folder: Folder | null = JSON.parse(await getUserFolderSelection()); + const folder: Folder | null = JSON.parse(await getUserFolderSelection() || "null"); if (folder === null) return; await clearDefaultTemplates(folder.id); diff --git a/src/legacyTemplates.ts b/src/legacyTemplates.ts index 15e9cba..3e7d764 100644 --- a/src/legacyTemplates.ts +++ b/src/legacyTemplates.ts @@ -25,8 +25,8 @@ const getTemplatesTag = async (): Promise => { export const loadLegacyTemplates = async (dateAndTimeUtils: DateAndTimeUtils, profileDir: string): Promise => { const fs = joplin.require("fs-extra"); - let folderId = null; - let templatesTagId = null; + let folderId: string | null = null; + let templatesTagId: string | null = null; const templatesDir = `${profileDir}/templates`; diff --git a/src/parser.ts b/src/parser.ts index f95df9b..8234bce 100644 --- a/src/parser.ts +++ b/src/parser.ts @@ -4,6 +4,7 @@ import { Logger } from "./logger"; import { DateAndTimeUtils } from "./utils/dateAndTime"; import { doesFolderExist } from "./utils/folders"; import { Note } from "./utils/templates"; +import { notEmpty } from "./utils/typescript"; import { getVariableFromDefinition } from "./variables/parser"; import { CustomVariable } from "./variables/types/base"; import { setTemplateVariablesView } from "./views/templateVariables"; @@ -23,6 +24,13 @@ export interface NewNote { todo_due: number | null; } +interface NoteMetadata { + title: string; + tags: string[]; + folder: string | null; + todo_due: number | null; +} + const NOTE_TITLE_VARIABLE_NAME = "template_title"; const NOTE_TAGS_VARIABLE_NAME = "template_tags"; const NOTE_FOLDER_VARIABLE_NAME = "template_notebook"; @@ -129,8 +137,8 @@ export class Parser { return res; } - private async getNoteMetadata(parsedSpecialVariables: Record) { - const meta = { + private async getNoteMetadata(parsedSpecialVariables: Record): Promise { + const meta: NoteMetadata = { title: parsedSpecialVariables.fallback_note_title, tags: [], folder: null, @@ -183,7 +191,7 @@ export class Parser { } else { return null; } - }).filter(val => !!val); + }).filter(notEmpty); if (!allMatchesAfterFirstMatch.length) { return null; @@ -229,7 +237,7 @@ export class Parser { return `${variableDefinitionsBlock}${templateContentBlock}`; } - public async parseTemplate(template: Note | null): Promise { + public async parseTemplate(template: Note | null): Promise { if (!template) { return null; } diff --git a/src/utils/dateAndTime.ts b/src/utils/dateAndTime.ts index 97e2c60..4588ed6 100644 --- a/src/utils/dateAndTime.ts +++ b/src/utils/dateAndTime.ts @@ -40,14 +40,14 @@ export class DateAndTimeUtils { return `${this.dateFormat} ${this.timeFormat}`; } - public formatMsToLocal(ms: number, format: string = null): string { + public formatMsToLocal(ms: number, format: string | null = null): string { if (!format) { format = this.getDateTimeFormat(); } return moment(ms).format(format); } - public formatLocalToJoplinCompatibleUnixTime(input: string, format: string = null): number { + public formatLocalToJoplinCompatibleUnixTime(input: string, format: string | null = null): number { if (!format) { format = this.getDateTimeFormat(); } @@ -60,7 +60,7 @@ export class DateAndTimeUtils { return date.unix() * 1000; } - public getCurrentTime(format: string = null): string { + public getCurrentTime(format: string | null = null): string { return this.formatMsToLocal(new Date().getTime(), format); } diff --git a/src/utils/typescript.ts b/src/utils/typescript.ts new file mode 100644 index 0000000..a839015 --- /dev/null +++ b/src/utils/typescript.ts @@ -0,0 +1,6 @@ +/** Can be used to filter non empty values from a collection while adhering to typescript + * strict null checks. + */ +export function notEmpty(value: T | null | undefined): value is T { + return value !== null && value !== undefined; +} diff --git a/src/variables/parser.ts b/src/variables/parser.ts index 77de6ae..a6d41bd 100644 --- a/src/variables/parser.ts +++ b/src/variables/parser.ts @@ -32,4 +32,8 @@ export const getVariableFromDefinition = (name: string, definition: unknown): Cu throw err; } } + + // This ideally should never happen. "InvalidCustomVariable" accepts + // all definitions. + throw Error("No valid definition for variable: " + name); } diff --git a/src/variables/types/base.ts b/src/variables/types/base.ts index c1ee834..b88a212 100644 --- a/src/variables/types/base.ts +++ b/src/variables/types/base.ts @@ -40,7 +40,7 @@ export class CustomVariable { static createFromDefinition(name: string, definition: unknown): CustomVariable { if (typeof definition === "string" && definition.trim() === this.definitionName) { return new this(name, name); - } else if (typeof definition === "object") { + } else if (typeof definition === "object" && definition !== null) { if ("type" in definition) { const variableType = definition["type"]; if (typeof variableType === "string" && variableType.trim() === this.definitionName) { diff --git a/src/variables/types/enum.ts b/src/variables/types/enum.ts index 3349d7b..ce5354e 100644 --- a/src/variables/types/enum.ts +++ b/src/variables/types/enum.ts @@ -35,7 +35,7 @@ export class EnumCustomVariable extends CustomVariable { if (typeof definition === "string") { const options = this.getOptionsFromType(definition); return new this(name, name, options); - } else if (typeof definition === "object") { + } else if (typeof definition === "object" && definition !== null) { if ("type" in definition) { const variableType = definition["type"]; if (typeof variableType === "string") { diff --git a/tests/parser.spec.ts b/tests/parser.spec.ts index 8097cc8..b7584bf 100644 --- a/tests/parser.spec.ts +++ b/tests/parser.spec.ts @@ -1,4 +1,6 @@ import joplin from "api"; + +import * as assert from "assert" import * as dedent from "dedent"; import { DateAndTimeUtils } from "@templates/utils/dateAndTime"; @@ -89,6 +91,7 @@ describe("Template parser", () => { ` }); + assert(parsedTemplate); expect(parsedTemplate.folder).toBeNull(); expect(parsedTemplate.tags.length).toEqual(0); expect(parsedTemplate.title).toEqual("Template Title"); @@ -129,13 +132,15 @@ describe("Template parser", () => { bows: NumberCustomVariable, show_summary: BooleanCustomVariable }); - handleVariableDialog("ok", { some_var: "text", bows: "12", show_summary: "false" }); + let parsedTemplate = await parser.parseTemplate(template); + + assert(parsedTemplate); expect(parsedTemplate.folder).toBeNull(); expect(parsedTemplate.tags.length).toEqual(0); expect(parsedTemplate.title).toEqual("Some Title"); @@ -151,7 +156,10 @@ describe("Template parser", () => { bows: "15", show_summary: "true" }); + parsedTemplate = await parser.parseTemplate(template); + + assert(parsedTemplate); expect(parsedTemplate.folder).toBeNull(); expect(parsedTemplate.tags.length).toEqual(0); expect(parsedTemplate.title).toEqual("Some Title"); @@ -183,12 +191,14 @@ describe("Template parser", () => { date_var: DateCustomVariable, time_var: TimeCustomVariable }); - handleVariableDialog("ok", { date_var: "2021-12-03", time_var: "20:43" }); + const parsedTemplate = await parser.parseTemplate(template); + + assert(parsedTemplate); expect(parsedTemplate.folder).toBeNull(); expect(parsedTemplate.tags.length).toEqual(0); expect(parsedTemplate.title).toEqual("Template"); @@ -212,11 +222,13 @@ describe("Template parser", () => { testVariableTypes({ dvar: EnumCustomVariable, }); - handleVariableDialog("ok", { dvar: "opt2", }); + const parsedTemplate = await parser.parseTemplate(template); + + assert(parsedTemplate); expect(parsedTemplate.folder).toBeNull(); expect(parsedTemplate.tags.length).toEqual(0); expect(parsedTemplate.title).toEqual("Template"); @@ -271,13 +283,15 @@ describe("Template parser", () => { expect(v.variable_two.toHTML()).toContain("Enter something below"); expect(v.variable_three.toHTML()).toContain("Choose an option"); }); - handleVariableDialog("ok", { variable_one: "val1", variable_two: "val2", variable_three: "opt1" }); + const parsedTemplate = await parser.parseTemplate(template); + + assert(parsedTemplate); expect(parsedTemplate.folder).toBeNull(); expect(parsedTemplate.tags.length).toEqual(0); expect(parsedTemplate.title).toEqual("Some Template"); @@ -327,7 +341,10 @@ describe("Template parser", () => { jest.spyOn(folderUtils, "doesFolderExist").mockImplementation(async (folderId: string) => { return folderId === "8e4d3851a1237028"; }); + const parsedTemplate = await parser.parseTemplate(template); + + assert(parsedTemplate); expect(parsedTemplate.folder).toEqual("8e4d3851a1237028"); expect(parsedTemplate.tags).toStrictEqual(["scrum", "Project 2"]); expect(parsedTemplate.title).toEqual("Scrum - Project 2 - Title 1"); @@ -441,7 +458,10 @@ describe("Template parser", () => { genre: "", status: "finished" }); + const parsedTemplate = await parser.parseTemplate(template); + + assert(parsedTemplate); expect(parsedTemplate.title).toEqual("Some Template"); expect(parsedTemplate.tags).toStrictEqual(["books", "finished"]); }); @@ -475,7 +495,10 @@ describe("Template parser", () => { some_date: "2023-05-09", some_time: "17:25" }); + const parsedTemplate = await parser.parseTemplate(template); + + assert(parsedTemplate); expect(parsedTemplate.folder).toBeNull(); expect(parsedTemplate.tags.length).toEqual(0); expect(parsedTemplate.title).toEqual("Some Template"); @@ -515,7 +538,10 @@ describe("Template parser", () => { num1: "11", num2: "4" }); + const parsedTemplate = await parser.parseTemplate(template); + + assert(parsedTemplate); expect(parsedTemplate.folder).toBeNull(); expect(parsedTemplate.tags.length).toEqual(0); expect(parsedTemplate.title).toEqual("Some Template"); @@ -531,7 +557,7 @@ describe("Template parser", () => { }); test("should show error with invalid usage of math helpers", async () => { - const invalidTemplates = []; + const invalidTemplates: string[] = []; invalidTemplates.push(dedent` --- num1: text @@ -613,7 +639,10 @@ describe("Template parser", () => { num1: "3", var1: "v" }); + const parsedTemplate = await parser.parseTemplate(template); + + assert(parsedTemplate); expect(parsedTemplate.folder).toBeNull(); expect(parsedTemplate.tags.length).toEqual(0); expect(parsedTemplate.title).toEqual("Some Template"); @@ -637,7 +666,7 @@ describe("Template parser", () => { }); test("should show error with invalid usage of repeat helper", async () => { - const invalidTemplates = []; + const invalidTemplates: string[] = []; invalidTemplates.push(dedent` --- var1: text @@ -707,7 +736,10 @@ describe("Template parser", () => { handleVariableDialog("ok", { var1: "Variable" }); + const parsedTemplate = await parser.parseTemplate(template); + + assert(parsedTemplate); expect(parsedTemplate.folder).toBeNull(); expect(parsedTemplate.tags.length).toEqual(0); expect(parsedTemplate.title).toEqual("Some Template"); @@ -719,7 +751,7 @@ describe("Template parser", () => { }); test("should show error with invalid usage of case helper", async () => { - const invalidTemplates = []; + const invalidTemplates: string[] = []; invalidTemplates.push(dedent` --- var1: text @@ -793,7 +825,10 @@ describe("Template parser", () => { var3: "40", var4: "false" }); + const parsedTemplate = await parser.parseTemplate(template); + + assert(parsedTemplate); expect(parsedTemplate.folder).toBeNull(); expect(parsedTemplate.tags.length).toEqual(0); expect(parsedTemplate.title).toEqual("Some Template"); @@ -812,7 +847,7 @@ describe("Template parser", () => { }); test("should show error with invalid usage of compare helper", async () => { - const invalidTemplates = []; + const invalidTemplates: string[] = []; invalidTemplates.push(dedent` --- var1: text @@ -879,7 +914,10 @@ describe("Template parser", () => { var2: "40", var3: "true", }); + const parsedTemplate = await parser.parseTemplate(template); + + assert(parsedTemplate); expect(parsedTemplate.folder).toBeNull(); expect(parsedTemplate.tags.length).toEqual(0); expect(parsedTemplate.title).toEqual("Some Template"); @@ -894,7 +932,7 @@ describe("Template parser", () => { }); test("should show error with invalid usage of condition helper", async () => { - const invalidTemplates = []; + const invalidTemplates: string[] = []; invalidTemplates.push(dedent` {{ condition false "~~" }} `); @@ -944,7 +982,10 @@ describe("Template parser", () => { handleVariableDialog("ok", { var1: "20", }); + const parsedTemplate = await parser.parseTemplate(template); + + assert(parsedTemplate); expect(parsedTemplate.folder).toBeNull(); expect(parsedTemplate.tags.length).toEqual(0); expect(parsedTemplate.title).toEqual("Some Template"); @@ -961,7 +1002,7 @@ describe("Template parser", () => { }); test("should show error with invalid usage of datetime helper", async () => { - const invalidTemplates = []; + const invalidTemplates: string[] = []; invalidTemplates.push(dedent` {{ datetime delta_hours="abc" }} `); @@ -1015,6 +1056,8 @@ describe("Template parser", () => { }); const parsedTemplate = await parser.parseTemplate(template); + + assert(parsedTemplate); expect(parsedTemplate.folder).toBeNull(); expect(parsedTemplate.tags.length).toEqual(0); expect(parsedTemplate.title).toEqual("Some Template"); diff --git a/tests/utils/templates.spec.ts b/tests/utils/templates.spec.ts index 711edbf..70ebf59 100644 --- a/tests/utils/templates.spec.ts +++ b/tests/utils/templates.spec.ts @@ -35,7 +35,7 @@ describe("Get user template selection", () => { } }); - const expectTemplatesSelector = (templates: DropdownOption[], selectedValue: DropdownOption) => { + const expectTemplatesSelector = (templates: DropdownOption[], selectedValue: DropdownOption | null) => { jest.spyOn(joplin.commands, "execute").mockImplementation(async (cmd: string, props: Record) => { expect(cmd).toEqual("showPrompt"); expect(props.autocomplete).toEqual(templates); @@ -62,6 +62,7 @@ describe("Get user template selection", () => { return tag.notes; } } + return []; }); } diff --git a/tsconfig.json b/tsconfig.json index 24ffe48..cbae57d 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -6,6 +6,7 @@ "jsx": "react", "allowJs": true, "baseUrl": ".", + "strictNullChecks": true, "paths": { "@templates/*": ["./src/*"] }