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.

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

add event bindings to no-incompatible-type-binding rule #101

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
3 changes: 3 additions & 0 deletions dev/src/my-element-1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@ export class MyElement extends LitElement {
return ["this is a test", "testing"];
}

didClick(evt: MouseEvent) {}

render() {
return html`
<button @click="${this.didClick}"></button>
<my-tsconfig-element size="large"></my-tsconfig-element>
<unknown-element @heheheh="${() => {}}" globalattribute></unknown-element>
<heheheh></heheheh>
Expand Down
25 changes: 4 additions & 21 deletions docs/readme/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down Expand Up @@ -296,26 +295,6 @@ The following example is not considered a warning:
html`<input .value="${text}" />`
```

#### 🌀 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:

<!-- prettier-ignore -->
```js
html`<button @click="${myEventHandler()}">Click</button>`
html`<button @click="${{hannndleEvent: console.log()}}">Click</button>`
```

The following examples are not considered warnings:

<!-- prettier-ignore -->
```js
html`<button @click="${myEventHandler}">Click</button>`
html`<button @click="${{handleEvent: console.log}}">Click</button>`
```

#### 😈 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.
Expand Down Expand Up @@ -386,6 +365,8 @@ html`<input type="wrongvalue" />`
html`<input placeholder />`
html`<input max="${"hello"}" />`
html`<my-list .listItems="${123}"></my-list>`
html`<button @click="${myEventHandler()}">Click</button>`
html`<button @click="${{hannndleEvent: console.log()}}">Click</button>`
```

The following examples are not considered warnings:
Expand All @@ -396,6 +377,8 @@ html`<input type="button" />`
html`<input placeholder="a placeholder" />`
html`<input max="${123}" />`
html`<my-list .listItems="${listItems}"></my-list>`
html`<button @click="${myEventHandler}">Click</button>`
html`<button @click="${{handleEvent: console.log}}">Click</button>`
```

#### 💥 no-invalid-directive-binding
Expand Down
25 changes: 4 additions & 21 deletions packages/lit-analyzer/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down Expand Up @@ -369,26 +368,6 @@ The following example is not considered a warning:
html`<input .value="${text}" />`
```

#### 🌀 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:

<!-- prettier-ignore -->
```js
html`<button @click="${myEventHandler()}">Click</button>`
html`<button @click="${{hannndleEvent: console.log()}}">Click</button>`
```

The following examples are not considered warnings:

<!-- prettier-ignore -->
```js
html`<button @click="${myEventHandler}">Click</button>`
html`<button @click="${{handleEvent: console.log}}">Click</button>`
```

#### 😈 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.
Expand Down Expand Up @@ -459,6 +438,8 @@ html`<input type="wrongvalue" />`
html`<input placeholder />`
html`<input max="${"hello"}" />`
html`<my-list .listItems="${123}"></my-list>`
html`<button @click="${myEventHandler()}">Click</button>`
html`<button @click="${{hannndleEvent: console.log()}}">Click</button>`
```

The following examples are not considered warnings:
Expand All @@ -469,6 +450,8 @@ html`<input type="button" />`
html`<input placeholder="a placeholder" />`
html`<input max="${123}" />`
html`<my-list .listItems="${listItems}"></my-list>`
html`<button @click="${myEventHandler}">Click</button>`
html`<button @click="${{handleEvent: console.log}}">Click</button>`
```

#### 💥 no-invalid-directive-binding
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,14 +227,21 @@ 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,
excludedDeclarationNames: ["HTMLElement"]
}
});

// 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
Expand Down
3 changes: 0 additions & 3 deletions packages/lit-analyzer/src/analyze/lit-analyzer-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -45,7 +44,6 @@ const DEFAULT_RULES_SEVERITY: Record<LitAnalyzerRuleId, [LitAnalyzerRuleSeverity
"no-unintended-mixed-binding": ["warn", "warn"],
"no-invalid-boolean-binding": ["error", "error"],
"no-expressionless-property-binding": ["error", "error"],
"no-noncallable-event-binding": ["error", "error"],
"no-boolean-in-attribute-binding": ["error", "error"],
"no-complex-attribute-binding": ["error", "error"],
"no-nullable-attribute-binding": ["error", "error"],
Expand Down Expand Up @@ -252,7 +250,6 @@ function getDeprecatedMappedRules(userOptions: Partial<LitAnalyzerConfig>): 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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
Expand Down Expand Up @@ -188,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
)}`;
}
Expand Down Expand Up @@ -256,7 +260,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<string, HtmlEvent>();
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[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -458,24 +458,34 @@ function mergeRelatedMembers<T extends HtmlMember>(members: Iterable<T>): 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 {
Expand Down Expand Up @@ -533,7 +543,7 @@ function mergeRelatedEvents(events: Iterable<HtmlEvent>): ReadonlyMap<string, Ht
...existingEvent,
global: existingEvent.global && event.global,
description: undefined,
getType: lazy(() => 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
});
Expand Down
2 changes: 0 additions & 2 deletions packages/lit-analyzer/src/rules/all-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -25,7 +24,6 @@ export const ALL_RULES: RuleModule[] = [
noExpressionlessPropertyBindingRule,
noUnintendedMixedBindingRule,
noUnknownSlotRule,
noNoncallableEventBindingRule,
noNullableAttributeBindingRule,
noComplexAttributeBindingRule,
noBooleanInAttributeBindingRule,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -37,6 +38,7 @@ const rule: RuleModule = {
break;

case LIT_HTML_EVENT_LISTENER_ATTRIBUTE_MODIFIER:
isAssignableInEventBinding(htmlAttr, { typeA, typeB }, context);
break;

default: {
Expand Down
67 changes: 0 additions & 67 deletions packages/lit-analyzer/src/rules/no-noncallable-event-binding.ts

This file was deleted.

Loading