Skip to content

feat: add a codefix to fix class to className in react & add spelling suggest for JSX attributes #37907

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

Merged
merged 15 commits into from
Jun 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,7 @@ namespace ts {
getAllPossiblePropertiesOfTypes,
getSuggestedSymbolForNonexistentProperty,
getSuggestionForNonexistentProperty,
getSuggestedSymbolForNonexistentJSXAttribute,
getSuggestedSymbolForNonexistentSymbol: (location, name, meaning) => getSuggestedSymbolForNonexistentSymbol(location, escapeLeadingUnderscores(name), meaning),
getSuggestionForNonexistentSymbol: (location, name, meaning) => getSuggestionForNonexistentSymbol(location, escapeLeadingUnderscores(name), meaning),
getSuggestedSymbolForNonexistentModule,
Expand Down Expand Up @@ -16313,18 +16314,25 @@ namespace ts {
if (isJsxAttributes(errorNode) || isJsxOpeningLikeElement(errorNode) || isJsxOpeningLikeElement(errorNode.parent)) {
// JsxAttributes has an object-literal flag and undergo same type-assignablity check as normal object-literal.
// However, using an object-literal error message will be very confusing to the users so we give different a message.
// TODO: Spelling suggestions for excess jsx attributes (needs new diagnostic messages)
if (prop.valueDeclaration && isJsxAttribute(prop.valueDeclaration) && getSourceFileOfNode(errorNode) === getSourceFileOfNode(prop.valueDeclaration.name)) {
// Note that extraneous children (as in `<NoChild>extra</NoChild>`) don't pass this check,
// since `children` is a SyntaxKind.PropertySignature instead of a SyntaxKind.JsxAttribute.
errorNode = prop.valueDeclaration.name;
}
reportError(Diagnostics.Property_0_does_not_exist_on_type_1, symbolToString(prop), typeToString(errorTarget));
const propName = symbolToString(prop);
const suggestionSymbol = getSuggestedSymbolForNonexistentJSXAttribute(propName, errorTarget);
const suggestion = suggestionSymbol ? symbolToString(suggestionSymbol) : undefined;
if (suggestion) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's technically the possibility of an empty symbol name here, but not clear when that comes up.

Also, as a nit, whenever we have an if/else pair we generally wrap those in curlies.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how to handle a symbol without name?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (suggestion !== undefined)

reportError(Diagnostics.Property_0_does_not_exist_on_type_1_Did_you_mean_2, propName, typeToString(errorTarget), suggestion);
}
else {
reportError(Diagnostics.Property_0_does_not_exist_on_type_1, propName, typeToString(errorTarget));
}
}
else {
// use the property's value declaration if the property is assigned inside the literal itself
const objectLiteralDeclaration = source.symbol && firstOrUndefined(source.symbol.declarations);
let suggestion;
let suggestion: string | undefined;
if (prop.valueDeclaration && findAncestor(prop.valueDeclaration, d => d === objectLiteralDeclaration) && getSourceFileOfNode(objectLiteralDeclaration) === getSourceFileOfNode(errorNode)) {
const propDeclaration = prop.valueDeclaration as ObjectLiteralElementLike;
Debug.assertNode(propDeclaration, isObjectLiteralElementLike);
Expand Down Expand Up @@ -24877,6 +24885,15 @@ namespace ts {
return getSpellingSuggestionForName(isString(name) ? name : idText(name), getPropertiesOfType(containingType), SymbolFlags.Value);
}

function getSuggestedSymbolForNonexistentJSXAttribute(name: Identifier | PrivateIdentifier | string, containingType: Type): Symbol | undefined {
const strName = isString(name) ? name : idText(name);
const properties = getPropertiesOfType(containingType);
const jsxSpecific = strName === "for" ? find(properties, x => symbolName(x) === "htmlFor")
: strName === "class" ? find(properties, x => symbolName(x) === "className")
: undefined;
return jsxSpecific ?? getSpellingSuggestionForName(strName, properties, SymbolFlags.Value);
}

function getSuggestionForNonexistentProperty(name: Identifier | PrivateIdentifier | string, containingType: Type): string | undefined {
const suggestion = getSuggestedSymbolForNonexistentProperty(name, containingType);
return suggestion && symbolName(suggestion);
Expand Down Expand Up @@ -28223,7 +28240,7 @@ namespace ts {
error(expr, Diagnostics.The_operand_of_a_delete_operator_must_be_a_property_reference);
return booleanType;
}
if (expr.kind === SyntaxKind.PropertyAccessExpression && isPrivateIdentifier(expr.name)) {
if (isPropertyAccessExpression(expr) && isPrivateIdentifier(expr.name)) {
error(expr, Diagnostics.The_operand_of_a_delete_operator_cannot_be_a_private_identifier);
}
const links = getNodeLinks(expr);
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4010,6 +4010,7 @@ namespace ts {
/* @internal */ tryGetMemberInModuleExportsAndProperties(memberName: string, moduleSymbol: Symbol): Symbol | undefined;
getApparentType(type: Type): Type;
/* @internal */ getSuggestedSymbolForNonexistentProperty(name: Identifier | PrivateIdentifier | string, containingType: Type): Symbol | undefined;
/* @internal */ getSuggestedSymbolForNonexistentJSXAttribute(name: Identifier | string, containingType: Type): Symbol | undefined;
/* @internal */ getSuggestionForNonexistentProperty(name: Identifier | PrivateIdentifier | string, containingType: Type): string | undefined;
/* @internal */ getSuggestedSymbolForNonexistentSymbol(location: Node, name: string, meaning: SymbolFlags): Symbol | undefined;
/* @internal */ getSuggestionForNonexistentSymbol(location: Node, name: string, meaning: SymbolFlags): string | undefined;
Expand Down
23 changes: 19 additions & 4 deletions src/services/codefixes/fixSpelling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,16 @@ namespace ts.codefix {
Diagnostics.Cannot_find_name_0_Did_you_mean_the_instance_member_this_0.code,
Diagnostics.Cannot_find_name_0_Did_you_mean_the_static_member_1_0.code,
Diagnostics.Module_0_has_no_exported_member_1_Did_you_mean_2.code,
// for JSX class components
Diagnostics.No_overload_matches_this_call.code,
// for JSX FC
Diagnostics.Type_0_is_not_assignable_to_type_1.code,
];
registerCodeFix({
errorCodes,
getCodeActions(context) {
const { sourceFile } = context;
const info = getInfo(sourceFile, context.span.start, context);
const { sourceFile, errorCode } = context;
const info = getInfo(sourceFile, context.span.start, context, errorCode);
if (!info) return undefined;
const { node, suggestedSymbol } = info;
const { target } = context.host.getCompilationSettings();
Expand All @@ -21,18 +25,23 @@ namespace ts.codefix {
},
fixIds: [fixId],
getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => {
const info = getInfo(diag.file, diag.start, context);
const info = getInfo(diag.file, diag.start, context, diag.code);
const { target } = context.host.getCompilationSettings();
if (info) doChange(changes, context.sourceFile, info.node, info.suggestedSymbol, target!);
}),
});

function getInfo(sourceFile: SourceFile, pos: number, context: CodeFixContextBase): { node: Node, suggestedSymbol: Symbol } | undefined {
function getInfo(sourceFile: SourceFile, pos: number, context: CodeFixContextBase, errorCode: number): { node: Node, suggestedSymbol: Symbol } | undefined {
// This is the identifier of the misspelled word. eg:
// this.speling = 1;
// ^^^^^^^
const node = getTokenAtPosition(sourceFile, pos);
const parent = node.parent;
// Only fix spelling for No_overload_matches_this_call emitted on the React class component
if ((
errorCode === Diagnostics.No_overload_matches_this_call.code ||
errorCode === Diagnostics.Type_0_is_not_assignable_to_type_1.code) &&
!isJsxAttribute(parent)) return undefined;
const checker = context.program.getTypeChecker();

let suggestedSymbol: Symbol | undefined;
Expand All @@ -52,6 +61,12 @@ namespace ts.codefix {
suggestedSymbol = checker.getSuggestedSymbolForNonexistentModule(node, resolvedSourceFile.symbol);
}
}
else if (isJsxAttribute(parent) && parent.name === node) {
Debug.assertNode(node, isIdentifier, "Expected an identifier for JSX attribute");
const tag = findAncestor(node, isJsxOpeningLikeElement)!;
const props = checker.getContextualTypeForArgumentAtIndex(tag, 0);
suggestedSymbol = checker.getSuggestedSymbolForNonexistentJSXAttribute(node, props!);
}
else {
const meaning = getMeaningFromLocation(node);
const name = getTextOfNode(node);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
tests/cases/compiler/spellingSuggestionJSXAttribute.tsx(8,4): error TS2322: Type '{ class: string; }' is not assignable to type 'DetailedHTMLProps<AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>'.
Property 'class' does not exist on type 'DetailedHTMLProps<AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>'. Did you mean 'className'?
tests/cases/compiler/spellingSuggestionJSXAttribute.tsx(9,4): error TS2322: Type '{ for: string; }' is not assignable to type 'DetailedHTMLProps<AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>'.
Property 'for' does not exist on type 'DetailedHTMLProps<AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>'.
tests/cases/compiler/spellingSuggestionJSXAttribute.tsx(10,8): error TS2322: Type '{ for: string; }' is not assignable to type 'DetailedHTMLProps<LabelHTMLAttributes<HTMLLabelElement>, HTMLLabelElement>'.
Property 'for' does not exist on type 'DetailedHTMLProps<LabelHTMLAttributes<HTMLLabelElement>, HTMLLabelElement>'. Did you mean 'htmlFor'?
tests/cases/compiler/spellingSuggestionJSXAttribute.tsx(11,8): error TS2322: Type '{ for: string; class: string; }' is not assignable to type 'DetailedHTMLProps<LabelHTMLAttributes<HTMLLabelElement>, HTMLLabelElement>'.
Property 'for' does not exist on type 'DetailedHTMLProps<LabelHTMLAttributes<HTMLLabelElement>, HTMLLabelElement>'. Did you mean 'htmlFor'?
tests/cases/compiler/spellingSuggestionJSXAttribute.tsx(12,9): error TS2769: No overload matches this call.
Overload 1 of 2, '(props: Readonly<{ className?: string; htmlFor?: string; }>): MyComp', gave the following error.
Type '{ class: string; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<MyComp> & Readonly<{ children?: ReactNode; }> & Readonly<{ className?: string; htmlFor?: string; }>'.
Property 'class' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<MyComp> & Readonly<{ children?: ReactNode; }> & Readonly<{ className?: string; htmlFor?: string; }>'. Did you mean 'className'?
Overload 2 of 2, '(props: { className?: string; htmlFor?: string; }, context?: any): MyComp', gave the following error.
Type '{ class: string; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<MyComp> & Readonly<{ children?: ReactNode; }> & Readonly<{ className?: string; htmlFor?: string; }>'.
Property 'class' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<MyComp> & Readonly<{ children?: ReactNode; }> & Readonly<{ className?: string; htmlFor?: string; }>'. Did you mean 'className'?
tests/cases/compiler/spellingSuggestionJSXAttribute.tsx(13,10): error TS2322: Type '{ class: string; }' is not assignable to type 'IntrinsicAttributes & { className?: string; htmlFor?: string; }'.
Property 'class' does not exist on type 'IntrinsicAttributes & { className?: string; htmlFor?: string; }'. Did you mean 'className'?
tests/cases/compiler/spellingSuggestionJSXAttribute.tsx(14,9): error TS2769: No overload matches this call.
Overload 1 of 2, '(props: Readonly<{ className?: string; htmlFor?: string; }>): MyComp', gave the following error.
Type '{ for: string; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<MyComp> & Readonly<{ children?: ReactNode; }> & Readonly<{ className?: string; htmlFor?: string; }>'.
Property 'for' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<MyComp> & Readonly<{ children?: ReactNode; }> & Readonly<{ className?: string; htmlFor?: string; }>'. Did you mean 'htmlFor'?
Overload 2 of 2, '(props: { className?: string; htmlFor?: string; }, context?: any): MyComp', gave the following error.
Type '{ for: string; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<MyComp> & Readonly<{ children?: ReactNode; }> & Readonly<{ className?: string; htmlFor?: string; }>'.
Property 'for' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<MyComp> & Readonly<{ children?: ReactNode; }> & Readonly<{ className?: string; htmlFor?: string; }>'. Did you mean 'htmlFor'?
tests/cases/compiler/spellingSuggestionJSXAttribute.tsx(15,10): error TS2322: Type '{ for: string; }' is not assignable to type 'IntrinsicAttributes & { className?: string; htmlFor?: string; }'.
Property 'for' does not exist on type 'IntrinsicAttributes & { className?: string; htmlFor?: string; }'. Did you mean 'htmlFor'?


==== tests/cases/compiler/spellingSuggestionJSXAttribute.tsx (8 errors) ====
/// <reference path="/.lib/react16.d.ts" />
import * as React from "react";

function MyComp2(props: { className?: string, htmlFor?: string }) {
return null!;
}
class MyComp extends React.Component<{ className?: string, htmlFor?: string }> { }
<a class="" />;
~~~~~
!!! error TS2322: Type '{ class: string; }' is not assignable to type 'DetailedHTMLProps<AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>'.
!!! error TS2322: Property 'class' does not exist on type 'DetailedHTMLProps<AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>'. Did you mean 'className'?
<a for="" />; // should have no fix
~~~
!!! error TS2322: Type '{ for: string; }' is not assignable to type 'DetailedHTMLProps<AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>'.
!!! error TS2322: Property 'for' does not exist on type 'DetailedHTMLProps<AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>'.
<label for="" />;
~~~
!!! error TS2322: Type '{ for: string; }' is not assignable to type 'DetailedHTMLProps<LabelHTMLAttributes<HTMLLabelElement>, HTMLLabelElement>'.
!!! error TS2322: Property 'for' does not exist on type 'DetailedHTMLProps<LabelHTMLAttributes<HTMLLabelElement>, HTMLLabelElement>'. Did you mean 'htmlFor'?
<label for="" class="" />;
~~~
!!! error TS2322: Type '{ for: string; class: string; }' is not assignable to type 'DetailedHTMLProps<LabelHTMLAttributes<HTMLLabelElement>, HTMLLabelElement>'.
!!! error TS2322: Property 'for' does not exist on type 'DetailedHTMLProps<LabelHTMLAttributes<HTMLLabelElement>, HTMLLabelElement>'. Did you mean 'htmlFor'?
<MyComp class="" />;
~~~~~
!!! error TS2769: No overload matches this call.
!!! error TS2769: Overload 1 of 2, '(props: Readonly<{ className?: string; htmlFor?: string; }>): MyComp', gave the following error.
!!! error TS2769: Type '{ class: string; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<MyComp> & Readonly<{ children?: ReactNode; }> & Readonly<{ className?: string; htmlFor?: string; }>'.
!!! error TS2769: Property 'class' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<MyComp> & Readonly<{ children?: ReactNode; }> & Readonly<{ className?: string; htmlFor?: string; }>'. Did you mean 'className'?
!!! error TS2769: Overload 2 of 2, '(props: { className?: string; htmlFor?: string; }, context?: any): MyComp', gave the following error.
!!! error TS2769: Type '{ class: string; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<MyComp> & Readonly<{ children?: ReactNode; }> & Readonly<{ className?: string; htmlFor?: string; }>'.
!!! error TS2769: Property 'class' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<MyComp> & Readonly<{ children?: ReactNode; }> & Readonly<{ className?: string; htmlFor?: string; }>'. Did you mean 'className'?
<MyComp2 class="" />;
~~~~~
!!! error TS2322: Type '{ class: string; }' is not assignable to type 'IntrinsicAttributes & { className?: string; htmlFor?: string; }'.
!!! error TS2322: Property 'class' does not exist on type 'IntrinsicAttributes & { className?: string; htmlFor?: string; }'. Did you mean 'className'?
<MyComp for="" />;
~~~
!!! error TS2769: No overload matches this call.
!!! error TS2769: Overload 1 of 2, '(props: Readonly<{ className?: string; htmlFor?: string; }>): MyComp', gave the following error.
!!! error TS2769: Type '{ for: string; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<MyComp> & Readonly<{ children?: ReactNode; }> & Readonly<{ className?: string; htmlFor?: string; }>'.
!!! error TS2769: Property 'for' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<MyComp> & Readonly<{ children?: ReactNode; }> & Readonly<{ className?: string; htmlFor?: string; }>'. Did you mean 'htmlFor'?
!!! error TS2769: Overload 2 of 2, '(props: { className?: string; htmlFor?: string; }, context?: any): MyComp', gave the following error.
!!! error TS2769: Type '{ for: string; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<MyComp> & Readonly<{ children?: ReactNode; }> & Readonly<{ className?: string; htmlFor?: string; }>'.
!!! error TS2769: Property 'for' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<MyComp> & Readonly<{ children?: ReactNode; }> & Readonly<{ className?: string; htmlFor?: string; }>'. Did you mean 'htmlFor'?
<MyComp2 for="" />;
~~~
!!! error TS2322: Type '{ for: string; }' is not assignable to type 'IntrinsicAttributes & { className?: string; htmlFor?: string; }'.
!!! error TS2322: Property 'for' does not exist on type 'IntrinsicAttributes & { className?: string; htmlFor?: string; }'. Did you mean 'htmlFor'?

Loading