Skip to content

Commit

Permalink
Add type checking support for the 'role' attribute and the 'controlsl…
Browse files Browse the repository at this point in the history
…ist' attribute. This fixes #89
  • Loading branch information
runem committed Jul 8, 2020
1 parent f39dd01 commit aa60afd
Show file tree
Hide file tree
Showing 6 changed files with 255 additions and 15 deletions.
130 changes: 125 additions & 5 deletions packages/lit-analyzer/src/analyze/data/extra-html-data.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { SimpleType, SimpleTypeStringLiteral } from "ts-simple-type";
import { SimpleType, SimpleTypeStringLiteral, SimpleTypeUnion } from "ts-simple-type";
import { makePrimitiveArrayType } from "../util/type-util";

const HTML_5_ATTR_TYPES: { [key: string]: string | string[] } = {
const HTML_5_ATTR_TYPES: { [key: string]: string | string[] | [string[]] } = {
onafterprint: "string",
onbeforeprint: "string",
onbeforeunload: "string",
Expand Down Expand Up @@ -41,6 +42,7 @@ const HTML_5_ATTR_TYPES: { [key: string]: string | string[] } = {
"aria-valuenow": "",
"aria-valuetext": "",
accesskey: "string",
translate: ["yes", "no", ""],
autocapitalize: ["off", "none", "on", "sentences", "words", "characters"],
class: "string",
contextmenu: "string",
Expand Down Expand Up @@ -119,7 +121,121 @@ const HTML_5_ATTR_TYPES: { [key: string]: string | string[] } = {
part: "string",
exportparts: "string",
theme: "string",
controlslist: "string"
controlslist: [["nodownload", "nofullscreen", "noremoteplayback"]],
role: [
[
"alert",
"alertdialog",
"button",
"checkbox",
"dialog",
"gridcell",
"link",
"log",
"marquee",
"menuitem",
"menuitemcheckbox",
"menuitemradio",
"option",
"progressbar",
"radio",
"scrollbar",
"searchbox",
"slider",
"spinbutton",
"status",
"switch",
"tab",
"tabpanel",
"textbox",
"timer",
"tooltip",
"treeitem",
"combobox",
"grid",
"listbox",
"menu",
"menubar",
"radiogroup",
"tablist",
"tree",
"treegrid",
"application",
"article",
"cell",
"columnheader",
"definition",
"directory",
"document",
"feed",
"figure",
"group",
"heading",
"img",
"list",
"listitem",
"math",
"none",
"note",
"presentation",
"region",
"row",
"rowgroup",
"rowheader",
"separator",
"table",
"term",
"text",
"toolbar",
"banner",
"complementary",
"contentinfo",
"form",
"main",
"navigation",
"region",
"search",
"doc-abstract",
"doc-acknowledgments",
"doc-afterword",
"doc-appendix",
"doc-backlink",
"doc-biblioentry",
"doc-bibliography",
"doc-biblioref",
"doc-chapter",
"doc-colophon",
"doc-conclusion",
"doc-cover",
"doc-credit",
"doc-credits",
"doc-dedication",
"doc-endnote",
"doc-endnotes",
"doc-epigraph",
"doc-epilogue",
"doc-errata",
"doc-example",
"doc-footnote",
"doc-foreword",
"doc-glossary",
"doc-glossref",
"doc-index",
"doc-introduction",
"doc-noteref",
"doc-notice",
"doc-pagebreak",
"doc-pagelist",
"doc-part",
"doc-preface",
"doc-prologue",
"doc-pullquote",
"doc-qna",
"doc-subtitle",
"doc-tip",
"doc-toc"
]
]
};

export function hasTypeForAttrName(attrName: string): boolean {
Expand All @@ -130,11 +246,15 @@ export function html5TagAttrType(attrName: string): SimpleType {
return stringToSimpleType(HTML_5_ATTR_TYPES[attrName] || "", attrName);
}

function stringToSimpleType(typeString: string | string[], name?: string): SimpleType {
function stringToSimpleType(typeString: string | string[] | [string[]], name?: string): SimpleType {
if (Array.isArray(typeString)) {
if (Array.isArray(typeString[0])) {
return makePrimitiveArrayType(stringToSimpleType(typeString[0]) as SimpleTypeUnion);
}

return {
kind: "UNION",
types: typeString.map(value => ({ kind: "STRING_LITERAL", value } as SimpleTypeStringLiteral))
types: (typeString as string[]).map(value => ({ kind: "STRING_LITERAL", value } as SimpleTypeStringLiteral))
};
}

Expand Down
4 changes: 2 additions & 2 deletions packages/lit-analyzer/src/analyze/types/range.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ export function makeDocumentPosition(position: number): SourceFilePosition {
}*/

// Ranges
export type DocumentRange = { start: DocumentOffset; end: DocumentOffset } & { _brand?: "document" };
export type DocumentRange = { start: DocumentOffset; end: DocumentOffset } & { _brand: "document" };

export type SourceFileRange = { start: SourceFilePosition; end: SourceFilePosition } & { _brand?: "sourcefile" };
export type SourceFileRange = { start: SourceFilePosition; end: SourceFilePosition } & { _brand: "sourcefile" };
12 changes: 6 additions & 6 deletions packages/lit-analyzer/src/analyze/util/rule-fix-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,10 @@ function ruleFixActionConverter(action: RuleFixAction): LitCodeFixAction[] {
if (existingModuleDeclaration == null) {
return [
{
range: {
range: makeSourceFileRange({
start: action.file.getEnd(),
end: action.file.getEnd()
},
}),
newText: MODULE_PART
}
];
Expand All @@ -170,10 +170,10 @@ function ruleFixActionConverter(action: RuleFixAction): LitCodeFixAction[] {
if (existingDeclaration == null) {
return [
{
range: {
range: makeSourceFileRange({
start: existingModuleBody.getStart() + 1,
end: existingModuleBody.getStart() + 1
},
}),
newText: DECLARATION_PART
}
];
Expand All @@ -182,10 +182,10 @@ function ruleFixActionConverter(action: RuleFixAction): LitCodeFixAction[] {
// If there is an existing declaration with "action.name", add members to it
return [
{
range: {
range: makeSourceFileRange({
start: existingDeclaration.name.getEnd() + 2,
end: existingDeclaration.name.getEnd() + 2
},
}),
newText: MEMBER_PART
}
];
Expand Down
28 changes: 28 additions & 0 deletions packages/lit-analyzer/src/analyze/util/type-util.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { SimpleType, SimpleTypeUnion } from "ts-simple-type";

const PRIMITIVE_STRING_ARRAY_TYPE_BRAND = Symbol("PRIMITIVE_STRING_ARRAY_TYPE");

/**
* Brands a union as a primitive array type
* This type is used for the "role" attribute that is a whitespace separated list
* @param union
*/
export function makePrimitiveArrayType(union: SimpleTypeUnion): SimpleTypeUnion {
const extendedUnion: SimpleTypeUnion = {
...union
};

// eslint-disable-next-line @typescript-eslint/no-explicit-any
(extendedUnion as any)[PRIMITIVE_STRING_ARRAY_TYPE_BRAND] = true;

return extendedUnion;
}

/**
* Returns if a simple type is branded as a primitive array type
* @param simpleType
*/
export function isPrimitiveArrayType(simpleType: SimpleType): simpleType is SimpleTypeUnion {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return simpleType.kind === "UNION" && (simpleType as any)[PRIMITIVE_STRING_ARRAY_TYPE_BRAND] === true;
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { isAssignableToType as _isAssignableToType, SimpleType, SimpleTypeComparisonOptions, typeToString } from "ts-simple-type";
import { HtmlNodeAttrAssignmentKind } from "../../../analyze/types/html-node/html-node-attr-assignment-types";
import { HtmlNodeAttrAssignment, HtmlNodeAttrAssignmentKind } from "../../../analyze/types/html-node/html-node-attr-assignment-types";
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 { documentRangeToSFRange, rangeFromHtmlNodeAttr } from "../../../analyze/util/range-util";
import { isPrimitiveArrayType } from "../../../analyze/util/type-util";
import { isLitDirective } from "../directive/is-lit-directive";
import { isAssignableBindingUnderSecuritySystem } from "./is-assignable-binding-under-security-system";
import { isAssignableToType } from "./is-assignable-to-type";
Expand Down Expand Up @@ -41,6 +42,11 @@ export function isAssignableInAttributeBinding(
}
}

const primitiveArrayTypeResult = isAssignableInPrimitiveArray(assignment, { typeA, typeB }, context);
if (primitiveArrayTypeResult !== undefined) {
return primitiveArrayTypeResult;
}

if (!isAssignableToType({ typeA, typeB }, context, { isAssignable: isAssignableToTypeWithStringCoercion })) {
context.report({
location: rangeFromHtmlNodeAttr(htmlAttr),
Expand Down Expand Up @@ -98,6 +104,9 @@ export function isAssignableToTypeWithStringCoercion(
);

case "STRING_LITERAL":
/*if (typeA.kind === "ARRAY" && typeA.type.kind === "STRING_LITERAL") {
}*/

// Take into account that the empty string is is equal to true
if (typeB.value.length === 0) {
if (_isAssignableToType(typeA, { kind: "BOOLEAN_LITERAL", value: true }, safeOptions)) {
Expand Down Expand Up @@ -184,3 +193,72 @@ export function isAssignableToTypeWithStringCoercion(

return undefined;
}

/**
* Certain attributes like "role" are string literals, but should be type checked
* by comparing each item in the white-space-separated array against typeA
* @param assignment
* @param typeA
* @param typeB
* @param context
*/
export function isAssignableInPrimitiveArray(
assignment: HtmlNodeAttrAssignment,
{ typeA, typeB }: { typeA: SimpleType; typeB: SimpleType },
context: RuleModuleContext
): boolean | undefined {
// Only check "STRING" and "EXPRESSION" for now
if (assignment.kind !== HtmlNodeAttrAssignmentKind.STRING && assignment.kind !== HtmlNodeAttrAssignmentKind.EXPRESSION) {
return undefined;
}

// Check if typeA is marked as a "primitive array type"
if (isPrimitiveArrayType(typeA) && typeB.kind === "STRING_LITERAL") {
// Split a value like: "button listitem" into ["button", " ", "listitem"]
const valuesAndWhitespace = typeB.value.split(/(\s+)/g);
const valuesNotAssignable: string[] = [];

const startOffset = assignment.location.start;
let offset = 0;

for (const value of valuesAndWhitespace) {
// Check all non-whitespace values
if (value.match(/\s+/) == null && value !== "") {
// Make sure that the the value is assignable to the union
if (
!isAssignableToType({ typeA, typeB: { kind: "STRING_LITERAL", value } }, context, { isAssignable: isAssignableToTypeWithStringCoercion })
) {
valuesNotAssignable.push(value);

// If the assignment kind is "STRING" we can report diagnostics directly on the value in the HTML
if (assignment.kind === "STRING") {
context.report({
location: documentRangeToSFRange(assignment.htmlAttr.document, {
start: startOffset + offset,
end: startOffset + offset + value.length
}),
message: `The value '${value}' is not assignable to '${typeToString(typeA)}'`
});
}
}
}

offset += value.length;
}

// If the assignment kind as "EXPRESSION" report a single diagnostic on the attribute name
if (assignment.kind === "EXPRESSION" && valuesNotAssignable.length > 0) {
const multiple = valuesNotAssignable.length > 1;
context.report({
location: rangeFromHtmlNodeAttr(assignment.htmlAttr),
message: `The value${multiple ? "s" : ""} ${valuesNotAssignable.map(v => `'${v}'`).join(", ")} ${
multiple ? "are" : "is"
} not assignable to '${typeToString(typeA)}'`
});
}

return valuesNotAssignable.length === 0;
}

return undefined;
}
14 changes: 14 additions & 0 deletions packages/lit-analyzer/test/rules/no-incompatible-type-binding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,3 +187,17 @@ html\`<input step="\${ifExists(10)}" />\`
`);
hasNoDiagnostics(t, diagnostics);
});

tsTest("Attribute binding: the role attribute is correctly type checked when given valid items", t => {
const { diagnostics } = getDiagnostics(`html\`<div role="button listitem"></div>\`
`);

hasNoDiagnostics(t, diagnostics);
});

tsTest("Attribute binding: the role attribute is correctly type checked when given invalid items", t => {
const { diagnostics } = getDiagnostics(`html\`<div role="button foo"></div>\`
`);

hasDiagnostic(t, diagnostics, "no-incompatible-type-binding");
});

0 comments on commit aa60afd

Please sign in to comment.