From 1e91dab50890271e7b9d83bcd0e31ab4c840f16d Mon Sep 17 00:00:00 2001 From: 43081j <43081j@users.noreply.github.com> Date: Sun, 10 May 2020 15:22:01 +0100 Subject: [PATCH 1/8] add event bindings to incompatible type binding rule --- .../src/rules/no-incompatible-type-binding.ts | 2 ++ .../type/is-assignable-in-event-binding.ts | 36 +++++++++++++++++++ .../test/helpers/generate-test-file.ts | 3 +- .../rules/no-incompatible-type-binding.ts | 19 ++++++++++ 4 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts diff --git a/packages/lit-analyzer/src/rules/no-incompatible-type-binding.ts b/packages/lit-analyzer/src/rules/no-incompatible-type-binding.ts index 016b0605..d4fb2e7e 100644 --- a/packages/lit-analyzer/src/rules/no-incompatible-type-binding.ts +++ b/packages/lit-analyzer/src/rules/no-incompatible-type-binding.ts @@ -8,6 +8,7 @@ import { extractBindingTypes } from "./util/type/extract-binding-types"; import { isAssignableInAttributeBinding } from "./util/type/is-assignable-in-attribute-binding"; import { isAssignableInBooleanBinding } from "./util/type/is-assignable-in-boolean-binding"; import { isAssignableInPropertyBinding } from "./util/type/is-assignable-in-property-binding"; +import { isAssignableInEventBinding } from "./util/type/is-assignable-in-event-binding"; /** * This rule validate if the types of a binding are assignable. @@ -37,6 +38,7 @@ const rule: RuleModule = { break; case LIT_HTML_EVENT_LISTENER_ATTRIBUTE_MODIFIER: + isAssignableInEventBinding(htmlAttr, { typeA, typeB }, context); break; default: { diff --git a/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts b/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts new file mode 100644 index 00000000..6f93b40a --- /dev/null +++ b/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts @@ -0,0 +1,36 @@ +import { SimpleType, SimpleTypeKind, toTypeString } from "ts-simple-type"; +import { HtmlNodeAttr } from "../../../analyze/types/html-node/html-node-attr-types"; +import { RuleModuleContext } from "../../../analyze/types/rule/rule-module-context"; +import { rangeFromHtmlNodeAttr } from "../../../analyze/util/range-util"; +import { isAssignableToType } from "./is-assignable-to-type"; + +export function isAssignableInEventBinding( + htmlAttr: HtmlNodeAttr, + { typeA, typeB }: { typeA: SimpleType; typeB: SimpleType }, + context: RuleModuleContext +): boolean | undefined { + const expectedType: SimpleType = { + kind: SimpleTypeKind.FUNCTION, + returnType: { kind: SimpleTypeKind.VOID }, + argTypes: [ + { + name: 'event', + type: typeA, + optional: false, + spread: false, + initializer: false + } + ] + }; + + if (!isAssignableToType({ typeA: expectedType, typeB }, context)) { + context.report({ + location: rangeFromHtmlNodeAttr(htmlAttr), + message: `Type '${toTypeString(typeB)}' is not assignable to '${toTypeString(expectedType)}'` + }); + + return false; + } + + return true; +} diff --git a/packages/lit-analyzer/test/helpers/generate-test-file.ts b/packages/lit-analyzer/test/helpers/generate-test-file.ts index c7d6b951..f2b9794a 100644 --- a/packages/lit-analyzer/test/helpers/generate-test-file.ts +++ b/packages/lit-analyzer/test/helpers/generate-test-file.ts @@ -1,11 +1,12 @@ import { TestFile } from "./compile-files"; -export function makeElement({ properties, slots }: { properties?: string[]; slots?: string[] }): TestFile { +export function makeElement({ properties, slots, events }: { properties?: string[]; slots?: string[]; events?: string[] }): TestFile { return { fileName: "my-element.ts", text: ` /** ${(slots || []).map(slot => ` * @slot ${slot}`)} +${(events || []).map(event => ` * @fires ${event}`)} */ class MyElement extends HTMLElement { ${(properties || []).map(prop => `@property() ${prop}`).join("\n")} diff --git a/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts b/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts index 29ecfa55..c73a0ed1 100644 --- a/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts +++ b/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts @@ -224,3 +224,22 @@ html\` \` hasDiagnostic(t, diagnostics, "no-incompatible-type-binding"); }); + +test("Event binding: event handler is assignable to valid event", t => { + const { diagnostics } = getDiagnostics([makeElement({ events: ["foo-event"] }), 'html` {}}>`']); + hasNoDiagnostics(t, diagnostics); +}); + +test("Event binding: event handler is assignable to valid typed event", t => { + const { diagnostics } = getDiagnostics([makeElement({ events: ["{MouseEvent} foo-event"] }), 'html` {}}>`']); + hasNoDiagnostics(t, diagnostics); +}); + +test("Event binding: invalid event handler is not assignable to typed event", t => { + const { diagnostics } = getDiagnostics([makeElement({ events: ["{MouseEvent} foo-event"] }), 'html` {}}>`']); + hasDiagnostic(t, diagnostics, "no-incompatible-type-binding"); +}); + +test("Event binding: invalid event handler is not assignable to event", t => { + const { diagnostics } = getDiagnostics([makeElement({ events: ["foo-event"] }), 'html` {}}>`']); +}); From 15898559bf27f34789b21aaca2d5b6825a0c53ef Mon Sep 17 00:00:00 2001 From: James Garbutt <43081j@users.noreply.github.com> Date: Sat, 11 Jul 2020 11:39:25 +0100 Subject: [PATCH 2/8] fix merge issues --- .../type/is-assignable-in-event-binding.ts | 14 +++++------ .../rules/no-incompatible-type-binding.ts | 25 +++++++++++++------ 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts b/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts index 6f93b40a..1a45aa32 100644 --- a/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts +++ b/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts @@ -1,4 +1,4 @@ -import { SimpleType, SimpleTypeKind, toTypeString } from "ts-simple-type"; +import { SimpleType, typeToString } from "ts-simple-type"; import { HtmlNodeAttr } from "../../../analyze/types/html-node/html-node-attr-types"; import { RuleModuleContext } from "../../../analyze/types/rule/rule-module-context"; import { rangeFromHtmlNodeAttr } from "../../../analyze/util/range-util"; @@ -10,14 +10,14 @@ export function isAssignableInEventBinding( context: RuleModuleContext ): boolean | undefined { const expectedType: SimpleType = { - kind: SimpleTypeKind.FUNCTION, - returnType: { kind: SimpleTypeKind.VOID }, - argTypes: [ + kind: "FUNCTION", + returnType: { kind: "VOID" }, + parameters: [ { - name: 'event', + name: "event", type: typeA, optional: false, - spread: false, + rest: false, initializer: false } ] @@ -26,7 +26,7 @@ export function isAssignableInEventBinding( if (!isAssignableToType({ typeA: expectedType, typeB }, context)) { context.report({ location: rangeFromHtmlNodeAttr(htmlAttr), - message: `Type '${toTypeString(typeB)}' is not assignable to '${toTypeString(expectedType)}'` + message: `Type '${typeToString(typeB)}' is not assignable to '${typeToString(expectedType)}'` }); return false; diff --git a/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts b/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts index c73a0ed1..bf708aa1 100644 --- a/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts +++ b/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts @@ -225,21 +225,30 @@ html\` \` hasDiagnostic(t, diagnostics, "no-incompatible-type-binding"); }); -test("Event binding: event handler is assignable to valid event", t => { - const { diagnostics } = getDiagnostics([makeElement({ events: ["foo-event"] }), 'html` {}}>`']); +tsTest("Event binding: event handler is assignable to valid event", t => { + const { diagnostics } = getDiagnostics([makeElement({ events: ["foo-event"] }), "html` {}}>`"]); hasNoDiagnostics(t, diagnostics); }); -test("Event binding: event handler is assignable to valid typed event", t => { - const { diagnostics } = getDiagnostics([makeElement({ events: ["{MouseEvent} foo-event"] }), 'html` {}}>`']); +tsTest("Event binding: event handler is assignable to valid typed event", t => { + const { diagnostics } = getDiagnostics([ + makeElement({ events: ["{MouseEvent} foo-event"] }), + "html` {}}>`" + ]); hasNoDiagnostics(t, diagnostics); }); -test("Event binding: invalid event handler is not assignable to typed event", t => { - const { diagnostics } = getDiagnostics([makeElement({ events: ["{MouseEvent} foo-event"] }), 'html` {}}>`']); +tsTest("Event binding: invalid event handler is not assignable to typed event", t => { + const { diagnostics } = getDiagnostics([ + makeElement({ events: ["{MouseEvent} foo-event"] }), + "html` {}}>`" + ]); hasDiagnostic(t, diagnostics, "no-incompatible-type-binding"); }); -test("Event binding: invalid event handler is not assignable to event", t => { - const { diagnostics } = getDiagnostics([makeElement({ events: ["foo-event"] }), 'html` {}}>`']); +tsTest("Event binding: invalid event handler is not assignable to event", t => { + const { diagnostics } = getDiagnostics([ + makeElement({ events: ["foo-event"] }), + "html` {}}>`" + ]); }); From b0399c09b96b1d95762203f25a0d5c5ec9742779 Mon Sep 17 00:00:00 2001 From: runem Date: Thu, 16 Jul 2020 11:41:43 +0200 Subject: [PATCH 3/8] Update isAssignableInEventBinding to handle non-typed events and take 'handleEvent' into account --- .../type/is-assignable-in-event-binding.ts | 44 ++++++++++++++----- .../rules/no-incompatible-type-binding.ts | 17 +++++++ 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts b/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts index 1a45aa32..07eb3933 100644 --- a/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts +++ b/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts @@ -1,4 +1,5 @@ import { SimpleType, typeToString } from "ts-simple-type"; +import { parseSimpleJsDocTypeExpression } from "web-component-analyzer"; import { HtmlNodeAttr } from "../../../analyze/types/html-node/html-node-attr-types"; import { RuleModuleContext } from "../../../analyze/types/rule/rule-module-context"; import { rangeFromHtmlNodeAttr } from "../../../analyze/util/range-util"; @@ -9,20 +10,41 @@ export function isAssignableInEventBinding( { typeA, typeB }: { typeA: SimpleType; typeB: SimpleType }, context: RuleModuleContext ): boolean | undefined { + // Treat type "ANY" as "Event" + if (typeA.kind === "ANY") { + typeA = parseSimpleJsDocTypeExpression("Event", context); + } + + // Construct an event handler type to perform type checking against const expectedType: SimpleType = { - kind: "FUNCTION", - returnType: { kind: "VOID" }, - parameters: [ - { - name: "event", - type: typeA, - optional: false, - rest: false, - initializer: false - } - ] + kind: "OBJECT", + call: { + kind: "FUNCTION", + returnType: { kind: "VOID" }, + parameters: [ + { + name: "event", + type: typeA, + optional: false, + rest: false, + initializer: false + } + ] + } }; + // Extract the type of "handleEvent" from typeB if an object was given to the event binding + switch (typeB.kind) { + case "OBJECT": + case "INTERFACE": { + const handleEventMember = typeB.members != null ? typeB.members.find(m => m.name === "handleEvent") : undefined; + if (handleEventMember != null) { + typeB = handleEventMember.type; + } + break; + } + } + if (!isAssignableToType({ typeA: expectedType, typeB }, context)) { context.report({ location: rangeFromHtmlNodeAttr(htmlAttr), diff --git a/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts b/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts index bf708aa1..3e25ab9b 100644 --- a/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts +++ b/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts @@ -251,4 +251,21 @@ tsTest("Event binding: invalid event handler is not assignable to event", t => { makeElement({ events: ["foo-event"] }), "html` {}}>`" ]); + hasDiagnostic(t, diagnostics, "no-incompatible-type-binding"); +}); + +tsTest("Event binding: invalid event handler (generic custom event) is not assignable to typed event", t => { + const { diagnostics } = getDiagnostics([ + makeElement({ events: ["{CustomEvent} foo-event"] }), + "html`) => {}}>`" + ]); + hasDiagnostic(t, diagnostics, "no-incompatible-type-binding"); +}); + +tsTest("Event binding: event handler (generic custom event) is assignable to typed event", t => { + const { diagnostics } = getDiagnostics([ + makeElement({ events: ["{CustomEvent} foo-event"] }), + "html`) => {}}>`" + ]); + hasNoDiagnostics(t, diagnostics); }); From 59e90ceec25f274e914665cb5fc101d12ecefc2f Mon Sep 17 00:00:00 2001 From: runem Date: Thu, 16 Jul 2020 19:41:45 +0200 Subject: [PATCH 4/8] Scan lib.dom.d.ts for global events. Fix problem where unknown event types would get incorrectly type checked --- dev/src/my-element-1.ts | 3 +++ .../analyze/default-lit-analyzer-context.ts | 9 ++++++- .../analyze/parse/parse-html-data/html-tag.ts | 27 ++++++++++++++++++- .../type/is-assignable-in-event-binding.ts | 13 ++++++++- .../rules/no-incompatible-type-binding.ts | 8 ++++++ 5 files changed, 57 insertions(+), 3 deletions(-) diff --git a/dev/src/my-element-1.ts b/dev/src/my-element-1.ts index 9afe595a..a82a4229 100644 --- a/dev/src/my-element-1.ts +++ b/dev/src/my-element-1.ts @@ -13,8 +13,11 @@ export class MyElement extends LitElement { return ["this is a test", "testing"]; } + didClick(evt: KeyboardEvent) {} + render() { return html` + diff --git a/packages/lit-analyzer/src/analyze/default-lit-analyzer-context.ts b/packages/lit-analyzer/src/analyze/default-lit-analyzer-context.ts index 3ec8a1c8..2a4798e2 100644 --- a/packages/lit-analyzer/src/analyze/default-lit-analyzer-context.ts +++ b/packages/lit-analyzer/src/analyze/default-lit-analyzer-context.ts @@ -227,7 +227,7 @@ export class DefaultLitAnalyzerContext implements LitAnalyzerContext { ts: this.ts, config: { features: ["event", "member", "slot", "csspart", "cssproperty"], - analyzeGlobalFeatures: !isDefaultLibrary, // Don't analyze global features in lib.dom.d.ts + analyzeGlobalFeatures: true, analyzeDefaultLib: true, analyzeDependencies: true, analyzeAllDeclarations: false, @@ -235,6 +235,13 @@ export class DefaultLitAnalyzerContext implements LitAnalyzerContext { } }); + // Don't analyze global members in lib.dom.d.ts for now + // We already merge in content from "HTMLElement", but in the future we might + // only use "globalFeatures" instead + if (isDefaultLibrary && analyzeResult.globalFeatures != null) { + analyzeResult.globalFeatures.members = []; + } + const reg = isDefaultLibrary ? HtmlDataSourceKind.BUILT_IN_DECLARED : HtmlDataSourceKind.DECLARED; // Forget diff --git a/packages/lit-analyzer/src/analyze/parse/parse-html-data/html-tag.ts b/packages/lit-analyzer/src/analyze/parse/parse-html-data/html-tag.ts index 8bbf4fe8..1e4a663f 100644 --- a/packages/lit-analyzer/src/analyze/parse/parse-html-data/html-tag.ts +++ b/packages/lit-analyzer/src/analyze/parse/parse-html-data/html-tag.ts @@ -2,6 +2,7 @@ import { isAssignableToSimpleTypeKind, SimpleType, typeToString } from "ts-simpl import { ComponentCssPart, ComponentCssProperty, ComponentDeclaration, ComponentEvent, ComponentMember, ComponentSlot } from "web-component-analyzer"; import { LIT_HTML_BOOLEAN_ATTRIBUTE_MODIFIER, LIT_HTML_EVENT_LISTENER_ATTRIBUTE_MODIFIER, LIT_HTML_PROP_ATTRIBUTE_MODIFIER } from "../../constants"; import { iterableDefined } from "../../util/iterable-util"; +import { lazy } from "../../util/general-util"; export interface HtmlDataFeatures { attributes: HtmlAttr[]; @@ -256,7 +257,31 @@ export function mergeHtmlProps(props: HtmlProp[]): HtmlProp[] { } export function mergeHtmlEvents(events: HtmlEvent[]): HtmlEvent[] { - return mergeFirstUnique(events, event => event.name); + if (events.length <= 1) { + return events; + } + + const mergedEvents = new Map(); + for (const evt of events) { + const existingEvent = mergedEvents.get(evt.name); + if (existingEvent != null) { + mergedEvents.set(evt.name, { + ...evt, + declaration: existingEvent.declaration || evt.declaration, + fromTagName: existingEvent.fromTagName || evt.fromTagName, + builtIn: existingEvent.builtIn || evt.builtIn, + global: existingEvent.global || evt.global, + description: existingEvent.description || evt.description, + getType: lazy(() => { + const type = existingEvent.getType(); + return type.kind === "ANY" ? evt.getType() : type; + }) + }); + } else { + mergedEvents.set(evt.name, evt); + } + } + return Array.from(mergedEvents.values()); } export function mergeHtmlSlots(slots: HtmlSlot[]): HtmlSlot[] { diff --git a/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts b/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts index 07eb3933..195cccf6 100644 --- a/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts +++ b/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts @@ -10,9 +10,20 @@ export function isAssignableInEventBinding( { typeA, typeB }: { typeA: SimpleType; typeB: SimpleType }, context: RuleModuleContext ): boolean | undefined { + let strictFunctionTypes: undefined | boolean = undefined; + // Treat type "ANY" as "Event" if (typeA.kind === "ANY") { typeA = parseSimpleJsDocTypeExpression("Event", context); + + // If we don't know the exact type of event required, always type check + // the event bivariant. In this case we only care if the parameter in + // typeB is an Event, and this is achieved forcing "strictFunctionTypes" + // to false (to have bivariant type checking for parameters) + // Examples on invalid error message we get if "strictFunctionTypes" is true: + // Type '(event: MouseEvent) => void' is not assignable to '(event: Event) => void' + // Type '(event: CustomEvent) => void' is not assignable to '(event: Event) => void' + strictFunctionTypes = false; } // Construct an event handler type to perform type checking against @@ -45,7 +56,7 @@ export function isAssignableInEventBinding( } } - if (!isAssignableToType({ typeA: expectedType, typeB }, context)) { + if (!isAssignableToType({ typeA: expectedType, typeB }, context, { strictFunctionTypes })) { context.report({ location: rangeFromHtmlNodeAttr(htmlAttr), message: `Type '${typeToString(typeB)}' is not assignable to '${typeToString(expectedType)}'` diff --git a/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts b/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts index 3e25ab9b..1669b084 100644 --- a/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts +++ b/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts @@ -269,3 +269,11 @@ tsTest("Event binding: event handler (generic custom event) is assignable to typ ]); hasNoDiagnostics(t, diagnostics); }); + +tsTest("Event binding: event handler is assignable to event with unknown type", t => { + const { diagnostics } = getDiagnostics([ + makeElement({ events: ["foo-event"] }), + "html` {}}>`" + ]); + hasNoDiagnostics(t, diagnostics); +}); From 7a9f5d5af3f943b02e0ff8de94f8ecc2baf03e14 Mon Sep 17 00:00:00 2001 From: runem Date: Thu, 16 Jul 2020 20:07:57 +0200 Subject: [PATCH 5/8] Show the event type in quick info for events --- .../src/analyze/parse/parse-html-data/html-tag.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/lit-analyzer/src/analyze/parse/parse-html-data/html-tag.ts b/packages/lit-analyzer/src/analyze/parse/parse-html-data/html-tag.ts index 1e4a663f..3726410a 100644 --- a/packages/lit-analyzer/src/analyze/parse/parse-html-data/html-tag.ts +++ b/packages/lit-analyzer/src/analyze/parse/parse-html-data/html-tag.ts @@ -189,7 +189,10 @@ export function documentationForHtmlTag(htmlTag: HtmlTag, options: DescriptionOp desc += `\n\n${descriptionList( "Events", items, - event => `${descriptionHeader(`@fires ${event.name}`, 0, options)}${event.description ? ` - ${event.description}` : ""}`, + event => + `${descriptionHeader(`@fires ${typeToString(event.getType())} ${event.name}`, 0, options)}${ + event.description ? ` - ${event.description}` : "" + }`, options )}`; } From 24d0ad95515282329ea5f141f0eb77dfd55a91df Mon Sep 17 00:00:00 2001 From: runem Date: Fri, 17 Jul 2020 18:54:42 +0200 Subject: [PATCH 6/8] Improve type merging and type checking for merged events --- dev/src/my-element-1.ts | 2 +- .../html-store/html-data-source-merged.ts | 36 ++++++++++++------- .../type/is-assignable-in-event-binding.ts | 15 +++++--- .../rules/no-incompatible-type-binding.ts | 9 +++++ 4 files changed, 44 insertions(+), 18 deletions(-) diff --git a/dev/src/my-element-1.ts b/dev/src/my-element-1.ts index a82a4229..574caae9 100644 --- a/dev/src/my-element-1.ts +++ b/dev/src/my-element-1.ts @@ -13,7 +13,7 @@ export class MyElement extends LitElement { return ["this is a test", "testing"]; } - didClick(evt: KeyboardEvent) {} + didClick(evt: MouseEvent) {} render() { return html` diff --git a/packages/lit-analyzer/src/analyze/store/html-store/html-data-source-merged.ts b/packages/lit-analyzer/src/analyze/store/html-store/html-data-source-merged.ts index ecf46b9e..3c6d3173 100644 --- a/packages/lit-analyzer/src/analyze/store/html-store/html-data-source-merged.ts +++ b/packages/lit-analyzer/src/analyze/store/html-store/html-data-source-merged.ts @@ -458,24 +458,34 @@ function mergeRelatedMembers(members: Iterable): Readon return mergedMembers; } -function mergeRelatedTypeToUnion(typeA: SimpleType, typeB: SimpleType): SimpleType { +function mergeRelatedTypeToUnion(typeA: SimpleType, typeB: SimpleType, { collapseAny = true }: { collapseAny?: boolean } = {}): SimpleType { if (typeA.kind === typeB.kind) { - switch (typeA.kind) { - case "ANY": - return typeA; + return typeA; + } + + if (collapseAny) { + if (typeB.kind === "ANY") { + return typeA; + } else if (typeA.kind === "ANY") { + return typeB; } } - switch (typeA.kind) { - case "UNION": - if (typeB.kind === "ANY" && typeA.types.find(t => t.kind === "ANY") != null) { + if (typeA.kind === "UNION" && typeB.kind === "UNION") { + if (collapseAny) { + if (typeA.types.some(t => t.kind === "ANY")) { + return typeB; + } + + if (typeB.types.some(t => t.kind === "ANY")) { return typeA; - } else { - return { - ...typeA, - types: [...typeA.types, typeB] - }; } + } + + return { + ...typeA, + types: Array.from(new Set([...typeA.types, ...typeB.types])) + }; } return { @@ -533,7 +543,7 @@ function mergeRelatedEvents(events: Iterable): ReadonlyMap mergeRelatedTypeToUnion(prevType(), event.getType())), + getType: lazy(() => mergeRelatedTypeToUnion(prevType(), event.getType(), { collapseAny: false })), related: existingEvent.related == null ? [existingEvent, event] : [...existingEvent.related, event], fromTagName: existingEvent.fromTagName || event.fromTagName }); diff --git a/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts b/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts index 195cccf6..e0dc7554 100644 --- a/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts +++ b/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts @@ -56,14 +56,21 @@ export function isAssignableInEventBinding( } } - if (!isAssignableToType({ typeA: expectedType, typeB }, context, { strictFunctionTypes })) { + let assignable: boolean; + if (typeA.kind === "UNION") { + // If events have been merged into one UNION type, check each event type separately + const mutedContext = { ...context, report() {} }; + assignable = typeA.types.some(tA => isAssignableInEventBinding(htmlAttr, { typeA: tA, typeB }, mutedContext)); + } else { + assignable = isAssignableToType({ typeA: expectedType, typeB }, context, { strictFunctionTypes }); + } + + if (!assignable) { context.report({ location: rangeFromHtmlNodeAttr(htmlAttr), message: `Type '${typeToString(typeB)}' is not assignable to '${typeToString(expectedType)}'` }); - - return false; } - return true; + return assignable; } diff --git a/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts b/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts index 1669b084..f5b71ee9 100644 --- a/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts +++ b/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts @@ -277,3 +277,12 @@ tsTest("Event binding: event handler is assignable to event with unknown type", ]); hasNoDiagnostics(t, diagnostics); }); + +tsTest("Event binding: event handler is assignable to merged event", t => { + const { diagnostics } = getDiagnostics([ + makeElement({ events: ["{MouseEvent} foo-event"] }), + makeElement({ events: ["{KeyboardEvent} foo-event"] }), + "html` {}}>`" + ]); + hasNoDiagnostics(t, diagnostics); +}); From e29177220201651b688967443e474a0de76500a5 Mon Sep 17 00:00:00 2001 From: runem Date: Fri, 17 Jul 2020 23:23:44 +0200 Subject: [PATCH 7/8] Remove no-noncallable-event-binding and move some logic into no-incompatible-type-binding --- docs/readme/rules.md | 25 ++----- packages/lit-analyzer/README.md | 25 ++----- .../src/analyze/lit-analyzer-config.ts | 3 - packages/lit-analyzer/src/rules/all-rules.ts | 2 - .../src/rules/no-noncallable-event-binding.ts | 67 ------------------- .../type/is-assignable-in-event-binding.ts | 55 ++++++++++++++- .../rules/no-incompatible-type-binding.ts | 55 +++++++++++++++ .../rules/no-noncallable-event-binding.ts | 58 ---------------- packages/ts-lit-plugin/README.md | 25 ++----- packages/vscode-lit-plugin/README.md | 25 ++----- packages/vscode-lit-plugin/package.json | 11 --- .../schemas/tsconfig.schema.json | 5 -- 12 files changed, 125 insertions(+), 231 deletions(-) delete mode 100644 packages/lit-analyzer/src/rules/no-noncallable-event-binding.ts delete mode 100644 packages/lit-analyzer/test/rules/no-noncallable-event-binding.ts diff --git a/docs/readme/rules.md b/docs/readme/rules.md index 2c1cd08c..a64e3caf 100644 --- a/docs/readme/rules.md +++ b/docs/readme/rules.md @@ -31,7 +31,6 @@ Each rule can have severity of `off`, `warning` or `error`. You can toggle rules | :------ | ----------- | --------------- | --------------- | | [no-invalid-boolean-binding](#-no-invalid-boolean-binding) | Disallow boolean attribute bindings on non-boolean types. | error | error | | [no-expressionless-property-binding](#-no-expressionless-property-binding) | Disallow property bindings without an expression. | error | error | -| [no-noncallable-event-binding](#-no-noncallable-event-binding) | Disallow event listener bindings with a noncallable type. | error | error | | [no-boolean-in-attribute-binding](#-no-boolean-in-attribute-binding) | Disallow attribute bindings with a boolean type. | error | error | | [no-complex-attribute-binding](#-no-complex-attribute-binding) | Disallow attribute bindings with a complex type. | error | error | | [no-nullable-attribute-binding](#-no-nullable-attribute-binding) | Disallow attribute bindings with nullable types such as "null" or "undefined". | error | error | @@ -296,26 +295,6 @@ The following example is not considered a warning: html`` ``` -#### 🌀 no-noncallable-event-binding - -It's a common mistake to incorrectly call the function when setting up an event handler binding instead of passing a reference to the function. This makes the function call whenever the code evaluates. - -The following examples are considered warnings: - - -```js -html`` -html`` -``` - -The following examples are not considered warnings: - - -```js -html`` -html`` -``` - #### 😈 no-boolean-in-attribute-binding You should not be binding to a boolean type using an attribute binding because it could result in binding the string "true" or "false". Instead you should be using a **boolean** attribute binding. @@ -386,6 +365,8 @@ html`` html`` html`` html`` +html`` +html`` ``` The following examples are not considered warnings: @@ -396,6 +377,8 @@ html`` html`` html`` html`` +html`` +html`` ``` #### 💥 no-invalid-directive-binding diff --git a/packages/lit-analyzer/README.md b/packages/lit-analyzer/README.md index b4f436fb..bd0e2ff6 100644 --- a/packages/lit-analyzer/README.md +++ b/packages/lit-analyzer/README.md @@ -104,7 +104,6 @@ Each rule can have severity of `off`, `warning` or `error`. You can toggle rules | :------ | ----------- | --------------- | --------------- | | [no-invalid-boolean-binding](#-no-invalid-boolean-binding) | Disallow boolean attribute bindings on non-boolean types. | error | error | | [no-expressionless-property-binding](#-no-expressionless-property-binding) | Disallow property bindings without an expression. | error | error | -| [no-noncallable-event-binding](#-no-noncallable-event-binding) | Disallow event listener bindings with a noncallable type. | error | error | | [no-boolean-in-attribute-binding](#-no-boolean-in-attribute-binding) | Disallow attribute bindings with a boolean type. | error | error | | [no-complex-attribute-binding](#-no-complex-attribute-binding) | Disallow attribute bindings with a complex type. | error | error | | [no-nullable-attribute-binding](#-no-nullable-attribute-binding) | Disallow attribute bindings with nullable types such as "null" or "undefined". | error | error | @@ -369,26 +368,6 @@ The following example is not considered a warning: html`` ``` -#### 🌀 no-noncallable-event-binding - -It's a common mistake to incorrectly call the function when setting up an event handler binding instead of passing a reference to the function. This makes the function call whenever the code evaluates. - -The following examples are considered warnings: - - -```js -html`` -html`` -``` - -The following examples are not considered warnings: - - -```js -html`` -html`` -``` - #### 😈 no-boolean-in-attribute-binding You should not be binding to a boolean type using an attribute binding because it could result in binding the string "true" or "false". Instead you should be using a **boolean** attribute binding. @@ -459,6 +438,8 @@ html`` html`` html`` html`` +html`` +html`` ``` The following examples are not considered warnings: @@ -469,6 +450,8 @@ html`` html`` html`` html`` +html`` +html`` ``` #### 💥 no-invalid-directive-binding diff --git a/packages/lit-analyzer/src/analyze/lit-analyzer-config.ts b/packages/lit-analyzer/src/analyze/lit-analyzer-config.ts index 5c590fd3..c24acfb8 100644 --- a/packages/lit-analyzer/src/analyze/lit-analyzer-config.ts +++ b/packages/lit-analyzer/src/analyze/lit-analyzer-config.ts @@ -14,7 +14,6 @@ export type LitAnalyzerRuleId = | "no-unintended-mixed-binding" | "no-invalid-boolean-binding" | "no-expressionless-property-binding" - | "no-noncallable-event-binding" | "no-boolean-in-attribute-binding" | "no-complex-attribute-binding" | "no-nullable-attribute-binding" @@ -45,7 +44,6 @@ const DEFAULT_RULES_SEVERITY: Record): LitA if (getDeprecatedOption(userOptions, "skipTypeChecking") === true) { Object.assign(mappedDeprecatedRules, { "no-invalid-boolean-binding": "off", - "no-noncallable-event-binding": "off", "no-boolean-in-attribute-binding": "off", "no-complex-attribute-binding": "off", "no-nullable-attribute-binding": "off", diff --git a/packages/lit-analyzer/src/rules/all-rules.ts b/packages/lit-analyzer/src/rules/all-rules.ts index 3e0b567f..66e77ab7 100644 --- a/packages/lit-analyzer/src/rules/all-rules.ts +++ b/packages/lit-analyzer/src/rules/all-rules.ts @@ -10,7 +10,6 @@ import noInvalidTagName from "./no-invalid-tag-name"; import noLegacyAttribute from "./no-legacy-attribute"; import noMissingElementTypeDefinition from "./no-missing-element-type-definition"; import noMissingImport from "./no-missing-import"; -import noNoncallableEventBindingRule from "./no-noncallable-event-binding"; import noNullableAttributeBindingRule from "./no-nullable-attribute-binding"; import noPropertyVisibilityMismatch from "./no-property-visibility-mismatch"; import noUnclosedTag from "./no-unclosed-tag"; @@ -25,7 +24,6 @@ export const ALL_RULES: RuleModule[] = [ noExpressionlessPropertyBindingRule, noUnintendedMixedBindingRule, noUnknownSlotRule, - noNoncallableEventBindingRule, noNullableAttributeBindingRule, noComplexAttributeBindingRule, noBooleanInAttributeBindingRule, diff --git a/packages/lit-analyzer/src/rules/no-noncallable-event-binding.ts b/packages/lit-analyzer/src/rules/no-noncallable-event-binding.ts deleted file mode 100644 index 6e8cb0f4..00000000 --- a/packages/lit-analyzer/src/rules/no-noncallable-event-binding.ts +++ /dev/null @@ -1,67 +0,0 @@ -import { isAssignableToSimpleTypeKind, SimpleType, typeToString, validateType } from "ts-simple-type"; -import { HtmlNodeAttrKind } from "../analyze/types/html-node/html-node-attr-types"; -import { RuleModule } from "../analyze/types/rule/rule-module"; -import { rangeFromHtmlNodeAttr } from "../analyze/util/range-util"; -import { extractBindingTypes } from "./util/type/extract-binding-types"; - -/** - * This rule validates that only callable types are used within event binding expressions. - * This rule catches typos like: @click="onClick()" - */ -const rule: RuleModule = { - id: "no-noncallable-event-binding", - meta: { - priority: "high" - }, - visitHtmlAssignment(assignment, context) { - // Only validate event listener bindings. - const { htmlAttr } = assignment; - if (htmlAttr.kind !== HtmlNodeAttrKind.EVENT_LISTENER) return; - - const { typeB } = extractBindingTypes(assignment, context); - - // Make sure that the expression given to the event listener binding a function or an object with "handleEvent" property. - if (!isTypeBindableToEventListener(typeB)) { - context.report({ - location: rangeFromHtmlNodeAttr(htmlAttr), - message: `You are setting up an event listener with a non-callable type '${typeToString(typeB)}'` - }); - } - } -}; - -export default rule; - -/** - * Returns if this type can be used in a event listener binding - * @param type - */ -function isTypeBindableToEventListener(type: SimpleType): boolean { - // Return "true" if the type has a call signature - if ("call" in type && type.call != null) { - return true; - } - - // Callable types can be used in the binding - if (isAssignableToSimpleTypeKind(type, ["FUNCTION", "METHOD", "UNKNOWN"], { matchAny: true })) { - return true; - } - - return validateType(type, simpleType => { - switch (simpleType.kind) { - // Object types with attributes for the setup function of the event listener can be used - case "OBJECT": - case "INTERFACE": { - // The "handleEvent" property must be present - const handleEventFunction = simpleType.members != null ? simpleType.members.find(m => m.name === "handleEvent") : undefined; - - // The "handleEvent" property must be callable - if (handleEventFunction != null) { - return isTypeBindableToEventListener(handleEventFunction.type); - } - } - } - - return undefined; - }); -} diff --git a/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts b/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts index e0dc7554..22486f45 100644 --- a/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts +++ b/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts @@ -1,4 +1,4 @@ -import { SimpleType, typeToString } from "ts-simple-type"; +import { isAssignableToSimpleTypeKind, SimpleType, typeToString, validateType } from "ts-simple-type"; import { parseSimpleJsDocTypeExpression } from "web-component-analyzer"; import { HtmlNodeAttr } from "../../../analyze/types/html-node/html-node-attr-types"; import { RuleModuleContext } from "../../../analyze/types/rule/rule-module-context"; @@ -9,6 +9,25 @@ export function isAssignableInEventBinding( htmlAttr: HtmlNodeAttr, { typeA, typeB }: { typeA: SimpleType; typeB: SimpleType }, context: RuleModuleContext +): boolean | undefined { + // Make sure that the expression given to the event listener binding a function or an object with "handleEvent" property. + // This catches typos like: @click="onClick()" + if (!isTypeBindableToEventListener(typeB)) { + context.report({ + location: rangeFromHtmlNodeAttr(htmlAttr), + message: `You are setting up an event listener with a non-callable type '${typeToString(typeB)}'` + }); + + return false; + } + + return isEventAssignable(htmlAttr, { typeA, typeB }, context); +} + +export function isEventAssignable( + htmlAttr: HtmlNodeAttr, + { typeA, typeB }: { typeA: SimpleType; typeB: SimpleType }, + context: RuleModuleContext ): boolean | undefined { let strictFunctionTypes: undefined | boolean = undefined; @@ -74,3 +93,37 @@ export function isAssignableInEventBinding( return assignable; } + +/** + * Returns if this type can be used in a event listener binding + * @param type + */ +function isTypeBindableToEventListener(type: SimpleType): boolean { + // Return "true" if the type has a call signature + if ("call" in type && type.call != null) { + return true; + } + + // Callable types can be used in the binding + if (isAssignableToSimpleTypeKind(type, ["FUNCTION", "METHOD", "UNKNOWN"], { matchAny: true })) { + return true; + } + + return validateType(type, simpleType => { + switch (simpleType.kind) { + // Object types with attributes for the setup function of the event listener can be used + case "OBJECT": + case "INTERFACE": { + // The "handleEvent" property must be present + const handleEventMember = simpleType.members != null ? simpleType.members.find(m => m.name === "handleEvent") : undefined; + + // The "handleEvent" property must be callable + if (handleEventMember != null) { + return isTypeBindableToEventListener(handleEventMember.type); + } + } + } + + return undefined; + }); +} diff --git a/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts b/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts index f5b71ee9..8791fcdb 100644 --- a/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts +++ b/packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts @@ -286,3 +286,58 @@ tsTest("Event binding: event handler is assignable to merged event", t => { ]); hasNoDiagnostics(t, diagnostics); }); + +tsTest("Event binding: Callable value is bindable", t => { + const { diagnostics } = getDiagnostics('html``'); + hasNoDiagnostics(t, diagnostics); +}); + +tsTest("Event binding: Non callback value is not bindable", t => { + const { diagnostics } = getDiagnostics('html``'); + hasDiagnostic(t, diagnostics, "no-incompatible-type-binding"); +}); + +tsTest("Event binding: Number is not bindable", t => { + const { diagnostics } = getDiagnostics('html``'); + hasDiagnostic(t, diagnostics, "no-incompatible-type-binding"); +}); + +tsTest("Event binding: Function is bindable", t => { + const { diagnostics } = getDiagnostics('function foo() {}; html``'); + hasNoDiagnostics(t, diagnostics); +}); + +tsTest("Event binding: Called function is not bindable", t => { + const { diagnostics } = getDiagnostics('function foo() {}; html``'); + hasDiagnostic(t, diagnostics, "no-incompatible-type-binding"); +}); + +tsTest("Event binding: Any type is bindable", t => { + const { diagnostics } = getDiagnostics('html``'); + hasNoDiagnostics(t, diagnostics); +}); + +tsTest("Event binding: Object with callable 'handleEvent' is bindable 1", t => { + const { diagnostics } = getDiagnostics('html``'); + hasNoDiagnostics(t, diagnostics); +}); + +tsTest("Event binding: Object with callable 'handleEvent' is bindable 2", t => { + const { diagnostics } = getDiagnostics('function foo() {}; html``'); + hasNoDiagnostics(t, diagnostics); +}); + +tsTest("Event binding: Object with called 'handleEvent' is not bindable", t => { + const { diagnostics } = getDiagnostics('function foo() {}; html``'); + hasDiagnostic(t, diagnostics, "no-incompatible-type-binding"); +}); + +tsTest("Event binding: Object literal without 'handleEvent' is not bindable", t => { + const { diagnostics } = getDiagnostics('function foo() {}; html``'); + hasDiagnostic(t, diagnostics, "no-incompatible-type-binding"); +}); + +tsTest("Event binding: Mixed value binding with first expression being callable is bindable", t => { + const { diagnostics } = getDiagnostics('html``'); + hasNoDiagnostics(t, diagnostics); +}); diff --git a/packages/lit-analyzer/test/rules/no-noncallable-event-binding.ts b/packages/lit-analyzer/test/rules/no-noncallable-event-binding.ts deleted file mode 100644 index 79744cee..00000000 --- a/packages/lit-analyzer/test/rules/no-noncallable-event-binding.ts +++ /dev/null @@ -1,58 +0,0 @@ -import { getDiagnostics } from "../helpers/analyze"; -import { hasDiagnostic, hasNoDiagnostics } from "../helpers/assert"; -import { tsTest } from "../helpers/ts-test"; - -tsTest("Event binding: Callable value is bindable", t => { - const { diagnostics } = getDiagnostics('html``'); - hasNoDiagnostics(t, diagnostics); -}); - -tsTest("Event binding: Non callback value is not bindable", t => { - const { diagnostics } = getDiagnostics('html``'); - hasDiagnostic(t, diagnostics, "no-noncallable-event-binding"); -}); - -tsTest("Event binding: Number is not bindable", t => { - const { diagnostics } = getDiagnostics('html``'); - hasDiagnostic(t, diagnostics, "no-noncallable-event-binding"); -}); - -tsTest("Event binding: Function is bindable", t => { - const { diagnostics } = getDiagnostics('function foo() {}; html``'); - hasNoDiagnostics(t, diagnostics); -}); - -tsTest("Event binding: Called function is not bindable", t => { - const { diagnostics } = getDiagnostics('function foo() {}; html``'); - hasDiagnostic(t, diagnostics, "no-noncallable-event-binding"); -}); - -tsTest("Event binding: Any type is bindable", t => { - const { diagnostics } = getDiagnostics('html``'); - hasNoDiagnostics(t, diagnostics); -}); - -tsTest("Event binding: Object with callable 'handleEvent' is bindable 1", t => { - const { diagnostics } = getDiagnostics('html``'); - hasNoDiagnostics(t, diagnostics); -}); - -tsTest("Event binding: Object with callable 'handleEvent' is bindable 2", t => { - const { diagnostics } = getDiagnostics('function foo() {}; html``'); - hasNoDiagnostics(t, diagnostics); -}); - -tsTest("Event binding: Object with called 'handleEvent' is not bindable", t => { - const { diagnostics } = getDiagnostics('function foo() {}; html``'); - hasDiagnostic(t, diagnostics, "no-noncallable-event-binding"); -}); - -tsTest("Event binding: Object literal without 'handleEvent' is not bindable", t => { - const { diagnostics } = getDiagnostics('function foo() {}; html``'); - hasDiagnostic(t, diagnostics, "no-noncallable-event-binding"); -}); - -tsTest("Event binding: Mixed value binding with first expression being callable is bindable", t => { - const { diagnostics } = getDiagnostics('html``'); - hasNoDiagnostics(t, diagnostics); -}); diff --git a/packages/ts-lit-plugin/README.md b/packages/ts-lit-plugin/README.md index a4905df5..ab09fe98 100644 --- a/packages/ts-lit-plugin/README.md +++ b/packages/ts-lit-plugin/README.md @@ -129,7 +129,6 @@ Each rule can have severity of `off`, `warning` or `error`. You can toggle rules | :------ | ----------- | --------------- | --------------- | | [no-invalid-boolean-binding](#-no-invalid-boolean-binding) | Disallow boolean attribute bindings on non-boolean types. | error | error | | [no-expressionless-property-binding](#-no-expressionless-property-binding) | Disallow property bindings without an expression. | error | error | -| [no-noncallable-event-binding](#-no-noncallable-event-binding) | Disallow event listener bindings with a noncallable type. | error | error | | [no-boolean-in-attribute-binding](#-no-boolean-in-attribute-binding) | Disallow attribute bindings with a boolean type. | error | error | | [no-complex-attribute-binding](#-no-complex-attribute-binding) | Disallow attribute bindings with a complex type. | error | error | | [no-nullable-attribute-binding](#-no-nullable-attribute-binding) | Disallow attribute bindings with nullable types such as "null" or "undefined". | error | error | @@ -394,26 +393,6 @@ The following example is not considered a warning: html`` ``` -#### 🌀 no-noncallable-event-binding - -It's a common mistake to incorrectly call the function when setting up an event handler binding instead of passing a reference to the function. This makes the function call whenever the code evaluates. - -The following examples are considered warnings: - - -```js -html`` -html`` -``` - -The following examples are not considered warnings: - - -```js -html`` -html`` -``` - #### 😈 no-boolean-in-attribute-binding You should not be binding to a boolean type using an attribute binding because it could result in binding the string "true" or "false". Instead you should be using a **boolean** attribute binding. @@ -484,6 +463,8 @@ html`` html`` html`` html`` +html`` +html`` ``` The following examples are not considered warnings: @@ -494,6 +475,8 @@ html`` html`` html`` html`` +html`` +html`` ``` #### 💥 no-invalid-directive-binding diff --git a/packages/vscode-lit-plugin/README.md b/packages/vscode-lit-plugin/README.md index 8dce4cf0..1c6d1925 100644 --- a/packages/vscode-lit-plugin/README.md +++ b/packages/vscode-lit-plugin/README.md @@ -65,7 +65,6 @@ Each rule can have severity of `off`, `warning` or `error`. You can toggle rules | :------ | ----------- | --------------- | --------------- | | [no-invalid-boolean-binding](#-no-invalid-boolean-binding) | Disallow boolean attribute bindings on non-boolean types. | error | error | | [no-expressionless-property-binding](#-no-expressionless-property-binding) | Disallow property bindings without an expression. | error | error | -| [no-noncallable-event-binding](#-no-noncallable-event-binding) | Disallow event listener bindings with a noncallable type. | error | error | | [no-boolean-in-attribute-binding](#-no-boolean-in-attribute-binding) | Disallow attribute bindings with a boolean type. | error | error | | [no-complex-attribute-binding](#-no-complex-attribute-binding) | Disallow attribute bindings with a complex type. | error | error | | [no-nullable-attribute-binding](#-no-nullable-attribute-binding) | Disallow attribute bindings with nullable types such as "null" or "undefined". | error | error | @@ -330,26 +329,6 @@ The following example is not considered a warning: html`` ``` -#### 🌀 no-noncallable-event-binding - -It's a common mistake to incorrectly call the function when setting up an event handler binding instead of passing a reference to the function. This makes the function call whenever the code evaluates. - -The following examples are considered warnings: - - -```js -html`` -html`` -``` - -The following examples are not considered warnings: - - -```js -html`` -html`` -``` - #### 😈 no-boolean-in-attribute-binding You should not be binding to a boolean type using an attribute binding because it could result in binding the string "true" or "false". Instead you should be using a **boolean** attribute binding. @@ -420,6 +399,8 @@ html`` html`` html`` html`` +html`` +html`` ``` The following examples are not considered warnings: @@ -430,6 +411,8 @@ html`` html`` html`` html`` +html`` +html`` ``` #### 💥 no-invalid-directive-binding diff --git a/packages/vscode-lit-plugin/package.json b/packages/vscode-lit-plugin/package.json index a1d5faa1..ce986f88 100644 --- a/packages/vscode-lit-plugin/package.json +++ b/packages/vscode-lit-plugin/package.json @@ -285,17 +285,6 @@ "error" ] }, - "lit-plugin.rules.no-noncallable-event-binding": { - "type": "string", - "description": "Disallow event listener bindings with a noncallable type.", - "default": "default", - "enum": [ - "default", - "off", - "warning", - "error" - ] - }, "lit-plugin.rules.no-boolean-in-attribute-binding": { "type": "string", "description": "Disallow attribute bindings with a boolean type.", diff --git a/packages/vscode-lit-plugin/schemas/tsconfig.schema.json b/packages/vscode-lit-plugin/schemas/tsconfig.schema.json index 74ed81cb..c0167537 100644 --- a/packages/vscode-lit-plugin/schemas/tsconfig.schema.json +++ b/packages/vscode-lit-plugin/schemas/tsconfig.schema.json @@ -148,11 +148,6 @@ "description": "Disallow property bindings without an expression.", "enum": ["off", "warning", "error"] }, - "no-noncallable-event-binding": { - "type": "string", - "description": "Disallow event listener bindings with a noncallable type.", - "enum": ["off", "warning", "error"] - }, "no-boolean-in-attribute-binding": { "type": "string", "description": "Disallow attribute bindings with a boolean type.", From b6e0eb5e5f5c0d2c7956544343fd42e536da97d7 Mon Sep 17 00:00:00 2001 From: runem Date: Fri, 17 Jul 2020 23:29:38 +0200 Subject: [PATCH 8/8] Always check events with strictFunctionTypes: false --- .../type/is-assignable-in-event-binding.ts | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts b/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts index 22486f45..8fe69092 100644 --- a/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts +++ b/packages/lit-analyzer/src/rules/util/type/is-assignable-in-event-binding.ts @@ -29,20 +29,9 @@ export function isEventAssignable( { typeA, typeB }: { typeA: SimpleType; typeB: SimpleType }, context: RuleModuleContext ): boolean | undefined { - let strictFunctionTypes: undefined | boolean = undefined; - // Treat type "ANY" as "Event" if (typeA.kind === "ANY") { typeA = parseSimpleJsDocTypeExpression("Event", context); - - // If we don't know the exact type of event required, always type check - // the event bivariant. In this case we only care if the parameter in - // typeB is an Event, and this is achieved forcing "strictFunctionTypes" - // to false (to have bivariant type checking for parameters) - // Examples on invalid error message we get if "strictFunctionTypes" is true: - // Type '(event: MouseEvent) => void' is not assignable to '(event: Event) => void' - // Type '(event: CustomEvent) => void' is not assignable to '(event: Event) => void' - strictFunctionTypes = false; } // Construct an event handler type to perform type checking against @@ -81,6 +70,17 @@ export function isEventAssignable( const mutedContext = { ...context, report() {} }; assignable = typeA.types.some(tA => isAssignableInEventBinding(htmlAttr, { typeA: tA, typeB }, mutedContext)); } else { + // We don't always know the exact type of event required. Therefore always type check + // the parameters bivariant instead of contravariant. We primarily care if the parameter in + // typeB is an Event (or something more specific), and this is achieved forcing "strictFunctionTypes" + // to false (to have bivariant type checking for parameters) + // Examples on invalid error message we get if "strictFunctionTypes" is true: + // Type '(event: MouseEvent) => void' is not assignable to '(event: Event) => void' + // Type '(event: CustomEvent) => void' is not assignable to '(event: Event) => void' + // In theory, we would be able to set "strictFunctionTypes: true" if we knew the exact type required, + // but under many circumstances we don't know exactly + const strictFunctionTypes = false; + assignable = isAssignableToType({ typeA: expectedType, typeB }, context, { strictFunctionTypes }); }