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

--noImplicitOverride #39669

Merged
merged 76 commits into from
Mar 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
76 commits
Select commit Hold shift + click to select a range
8b90b8d
wip: add types
Kingwl Jul 15, 2020
ea5608a
wip
Kingwl Jul 17, 2020
9371d87
Add cases
Kingwl Jul 17, 2020
6cb7f5e
Add some case
Kingwl Jul 17, 2020
061cd27
Add more check
Kingwl Jul 20, 2020
37ada55
accept baseline
Kingwl Jul 20, 2020
6401e1a
add abstract abd declare method
Kingwl Jul 20, 2020
be00363
add override in declaration
Kingwl Jul 20, 2020
fbb6e28
accept baseline
Kingwl Jul 20, 2020
6014a3a
add property override
Kingwl Jul 21, 2020
a65df91
Fix decalre modifier
Kingwl Jul 21, 2020
43d9f70
update baseline
Kingwl Jul 21, 2020
e73c941
Add more cases
Kingwl Jul 21, 2020
c0edf6f
make lint happy
Kingwl Jul 21, 2020
dca93ac
make lint happy
Kingwl Jul 21, 2020
2fd560a
Update description
Kingwl Jul 21, 2020
94187c7
Add codefix
Kingwl Aug 19, 2020
2b57fbd
simplify code
Kingwl Aug 19, 2020
3a2286b
Merge branch 'master' into exprement-override
Kingwl Aug 19, 2020
d34afb9
accept baseline
Kingwl Aug 19, 2020
b4614d1
Update desc
Kingwl Aug 21, 2020
1ef034d
Accept baseline
Kingwl Aug 21, 2020
aebfb28
Add override completions
Kingwl Aug 24, 2020
7753983
filter out implements field in override context
Kingwl Aug 24, 2020
5c04ccc
fix tests
Kingwl Aug 24, 2020
add16c9
Add parameter property check
Kingwl Aug 24, 2020
5316e2f
Accept baseline
Kingwl Aug 24, 2020
f36fede
acept baseline
Kingwl Aug 24, 2020
811bd1e
Add parameter property to declaration code action
Kingwl Aug 25, 2020
d0f5095
Add quickfix for override parameter property
Kingwl Aug 25, 2020
dd51815
Merge branch 'master' into exprement-override
Kingwl Sep 1, 2020
22bb09d
fix code style
Kingwl Sep 3, 2020
eee3b77
Add override with interface tests
Kingwl Sep 3, 2020
f94e93c
Add more cases about modifier position
Kingwl Sep 3, 2020
74f14c3
Merge branch 'master' into exprement-override
Kingwl Sep 14, 2020
e35ee17
rename flag
Kingwl Sep 14, 2020
d8fda5f
rename flags
Kingwl Sep 14, 2020
18434fd
Added tests.
DanielRosenwasser Sep 14, 2020
5ab5a7f
Accepted baselines.
DanielRosenwasser Sep 15, 2020
f52822a
Always issue errors for unnecessary 'override' modifiers.
DanielRosenwasser Sep 15, 2020
48c23a5
Accepted baselines.
DanielRosenwasser Sep 15, 2020
a3f0443
Merge branch 'master' into exprement-override
Kingwl Sep 15, 2020
cdd0739
Override perf (#4)
Kingwl Sep 15, 2020
8e53ad3
Merge branch 'master' into exprement-override
Kingwl Sep 16, 2020
827bc95
Do not issue error if implement abstract
Kingwl Sep 16, 2020
618f883
Add abstract-spec check
Kingwl Sep 16, 2020
bc4d758
Avoid override dead lock
Kingwl Sep 16, 2020
47296d6
Add more case
Kingwl Sep 16, 2020
5d19a90
Add codefix for new error
Kingwl Sep 16, 2020
68e1440
Fix error message
Kingwl Sep 16, 2020
82109e9
Merge branch 'master' into exprement-override
Kingwl Oct 30, 2020
2552495
Add jsdoc override tag (#6)
Kingwl Nov 2, 2020
cf5dc7e
Override jsdoc tag (#7)
Kingwl Nov 2, 2020
b65d244
Disallow codefix in js
Kingwl Nov 2, 2020
84cc69d
Merge branch 'master' into exprement-override
Kingwl Nov 5, 2020
6d89a7a
update baseline
Kingwl Nov 5, 2020
062f149
update baseline
Kingwl Nov 5, 2020
feb9093
Omit override in d.ts
Kingwl Nov 5, 2020
0c09a2e
Add more case in js
Kingwl Nov 5, 2020
3cf3c28
Accept baseline
Kingwl Nov 5, 2020
d874ae8
fix override js test
Kingwl Nov 6, 2020
e361ddc
Merge branch 'master' into exprement-override
Kingwl Dec 31, 2020
40da344
Merge branch 'master' into exprement-override
Kingwl Jan 7, 2021
f9384bc
fix crlf
Kingwl Jan 7, 2021
eea8259
Revert merge conflict changes
Kingwl Jan 7, 2021
ff3cbc1
Accept baseline
Kingwl Jan 7, 2021
a3766d2
Avoid space
Kingwl Jan 7, 2021
e2c245e
Merge branch 'master' into exprement-override
Kingwl Feb 4, 2021
8f3dcb9
Merge branch 'master' into exprement-override
Kingwl Mar 5, 2021
1db64fb
Fix CR issues
Kingwl Mar 5, 2021
71c00d5
Merge branch 'master' into exprement-override
Kingwl Mar 8, 2021
e3c0993
Accept baseline
Kingwl Mar 8, 2021
89517a0
Fix typo and add more check
Kingwl Mar 9, 2021
6c8d00d
Fix error name
Kingwl Mar 9, 2021
bbfb7eb
Merge branch 'master' into exprement-override
Kingwl Mar 10, 2021
4575b5a
Merge branch 'master' into exprement-override
Kingwl Mar 26, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
90 changes: 88 additions & 2 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36618,7 +36618,8 @@ namespace ts {
checkClassForDuplicateDeclarations(node);

// Only check for reserved static identifiers on non-ambient context.
if (!(node.flags & NodeFlags.Ambient)) {
const nodeInAmbientContext = !!(node.flags & NodeFlags.Ambient);
if (!nodeInAmbientContext) {
checkClassForStaticPropertyNameConflicts(node);
}

Expand Down Expand Up @@ -36682,6 +36683,8 @@ namespace ts {
}
}

checkMembersForMissingOverrideModifier(node, type, typeWithThis);

const implementedTypeNodes = getEffectiveImplementsTypeNodes(node);
if (implementedTypeNodes) {
for (const typeRefNode of implementedTypeNodes) {
Expand Down Expand Up @@ -36716,6 +36719,60 @@ namespace ts {
}
}

function checkMembersForMissingOverrideModifier(node: ClassLikeDeclaration, type: InterfaceType, typeWithThis: Type) {
const nodeInAmbientContext = !!(node.flags & NodeFlags.Ambient);
const baseTypeNode = getEffectiveBaseTypeNode(node);
const baseTypes = baseTypeNode && getBaseTypes(type);
const baseWithThis = baseTypes?.length ? getTypeWithThisArgument(first(baseTypes), type.thisType) : undefined;

for (const member of node.members) {
if (isConstructorDeclaration(member)) {
forEach(member.parameters, param => {
if (isParameterPropertyDeclaration(param, member)) {
checkClassMember(param, /*memberIsParameterProperty*/ true);
}
});
}
checkClassMember(member);
}
function checkClassMember(member: ClassElement | ParameterPropertyDeclaration, memberIsParameterProperty?: boolean) {
const hasOverride = hasOverrideModifier(member);
if (baseWithThis && (hasOverride || compilerOptions.noImplicitOverride)) {
const declaredProp = member.name && getSymbolAtLocation(member.name) || getSymbolAtLocation(member);
if (!declaredProp) {
return;
}

const baseClassName = typeToString(baseWithThis);
const prop = getPropertyOfType(typeWithThis, declaredProp.escapedName);
const baseProp = getPropertyOfType(baseWithThis, declaredProp.escapedName);
if (prop && !baseProp && hasOverride) {
error(member, Diagnostics.This_member_cannot_have_an_override_modifier_because_it_is_not_declared_in_the_base_class_0, baseClassName);
}
else if (prop && baseProp?.valueDeclaration && compilerOptions.noImplicitOverride && !nodeInAmbientContext) {
const baseHasAbstract = hasAbstractModifier(baseProp.valueDeclaration);
if (hasOverride) {
return;
}

if (!baseHasAbstract) {
const diag = memberIsParameterProperty ?
Diagnostics.This_parameter_property_must_be_rewritten_as_a_property_declaration_with_an_override_modifier_because_it_overrides_a_member_in_base_class_0 :
Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_a_member_in_the_base_class_0;
error(member, diag, baseClassName);
}
else if (hasAbstractModifier(member) && baseHasAbstract) {
error(member, Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_an_abstract_method_that_is_declared_in_the_base_class_0, baseClassName);
}
}
}
else if (hasOverride) {
const className = typeToString(type);
error(member, Diagnostics.This_member_cannot_have_an_override_modifier_because_its_containing_class_0_does_not_extend_another_class, className);
}
}
}

function issueMemberSpecificError(node: ClassLikeDeclaration, typeWithThis: Type, baseWithThis: Type, broadDiag: DiagnosticMessage) {
// iterate over all implemented properties and issue errors on each one which isn't compatible, rather than the class as a whole, if possible
let issuedMemberError = false;
Expand Down Expand Up @@ -40090,7 +40147,7 @@ namespace ts {
return quickResult;
}

let lastStatic: Node | undefined, lastDeclare: Node | undefined, lastAsync: Node | undefined, lastReadonly: Node | undefined;
let lastStatic: Node | undefined, lastDeclare: Node | undefined, lastAsync: Node | undefined, lastReadonly: Node | undefined, lastOverride: Node | undefined;
let flags = ModifierFlags.None;
for (const modifier of node.modifiers!) {
if (modifier.kind !== SyntaxKind.ReadonlyKeyword) {
Expand All @@ -40107,6 +40164,23 @@ namespace ts {
return grammarErrorOnNode(node, Diagnostics.A_class_member_cannot_have_the_0_keyword, tokenToString(SyntaxKind.ConstKeyword));
}
break;
case SyntaxKind.OverrideKeyword:
if (flags & ModifierFlags.Override) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_already_seen, "override");
}
else if (flags & ModifierFlags.Ambient) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_be_used_with_1_modifier, "override", "declare");
}
else if (flags & ModifierFlags.Static) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_be_used_with_1_modifier, "static", "override");
}
if (node.kind === SyntaxKind.Parameter) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_appear_on_a_parameter, "override");
}
flags |= ModifierFlags.Override;
lastOverride = modifier;
break;

case SyntaxKind.PublicKeyword:
case SyntaxKind.ProtectedKeyword:
case SyntaxKind.PrivateKeyword:
Expand All @@ -40115,6 +40189,9 @@ namespace ts {
if (flags & ModifierFlags.AccessibilityModifier) {
return grammarErrorOnNode(modifier, Diagnostics.Accessibility_modifier_already_seen);
}
else if (compilerOptions.noImplicitOverride && flags & ModifierFlags.Override) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_must_precede_1_modifier, text, "override");
}
else if (flags & ModifierFlags.Static) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_must_precede_1_modifier, text, "static");
}
Expand Down Expand Up @@ -40148,6 +40225,9 @@ namespace ts {
else if (flags & ModifierFlags.Readonly) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_must_precede_1_modifier, "static", "readonly");
}
else if (compilerOptions.noImplicitOverride && flags & ModifierFlags.Override) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_be_used_with_1_modifier, "static", "override");
}
else if (flags & ModifierFlags.Async) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_must_precede_1_modifier, "static", "async");
}
Expand Down Expand Up @@ -40212,6 +40292,9 @@ namespace ts {
else if (flags & ModifierFlags.Async) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_be_used_in_an_ambient_context, "async");
}
else if (compilerOptions.noImplicitOverride && flags & ModifierFlags.Override) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_be_used_in_an_ambient_context, "override");
}
else if (isClassLike(node.parent) && !isPropertyDeclaration(node)) {
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_appear_on_class_elements_of_this_kind, "declare");
}
Expand Down Expand Up @@ -40286,6 +40369,9 @@ namespace ts {
if (flags & ModifierFlags.Abstract) {
return grammarErrorOnNode(lastStatic!, Diagnostics._0_modifier_cannot_appear_on_a_constructor_declaration, "abstract"); // TODO: GH#18217
}
if (compilerOptions.noImplicitOverride && flags & ModifierFlags.Override) {
return grammarErrorOnNode(lastOverride!, Diagnostics._0_modifier_cannot_appear_on_a_constructor_declaration, "override"); // TODO: GH#18217
}
else if (flags & ModifierFlags.Async) {
return grammarErrorOnNode(lastAsync!, Diagnostics._0_modifier_cannot_appear_on_a_constructor_declaration, "async");
}
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/commandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,14 @@ namespace ts {
category: Diagnostics.Additional_Checks,
description: Diagnostics.Include_undefined_in_index_signature_results
},
{
name: "noImplicitOverride",
type: "boolean",
affectsSemanticDiagnostics: true,
showInSimplifiedHelpView: false,
category: Diagnostics.Additional_Checks,
description: Diagnostics.Ensure_overriding_members_in_derived_classes_are_marked_with_an_override_modifier
},
{
name: "noPropertyAccessFromIndexSignature",
type: "boolean",
Expand Down
48 changes: 44 additions & 4 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3685,6 +3685,26 @@
"category": "Error",
"code": 4111
},
"This member cannot have an 'override' modifier because its containing class '{0}' does not extend another class.": {
"category": "Error",
"code": 4112
},
"This member cannot have an 'override' modifier because it is not declared in the base class '{0}'.": {
"category": "Error",
"code": 4113
},
"This member must have an 'override' modifier because it overrides a member in the base class '{0}'.": {
"category": "Error",
"code": 4114
},
"This parameter property must be rewritten as a property declaration with an 'override' modifier because it overrides a member in base class '{0}'.": {
"category": "Error",
"code": 4115
},
"This member must have an 'override' modifier because it overrides an abstract method that is declared in the base class '{0}'.": {
"category": "Error",
"code": 4116
},

"The current host does not support the '{0}' option.": {
"category": "Error",
Expand Down Expand Up @@ -5018,15 +5038,19 @@
"category": "Message",
"code": 6505
},
"Require undeclared properties from index signatures to use element accesses.": {
"category": "Error",
"code": 6803
},

"Include 'undefined' in index signature results": {
"category": "Message",
"code": 6800
},
"Ensure overriding members in derived classes are marked with an 'override' modifier.": {
"category": "Message",
"code": 6801
},
"Require undeclared properties from index signatures to use element accesses.": {
"category": "Message",
"code": 6802
},

"Variable '{0}' implicitly has an '{1}' type.": {
"category": "Error",
Expand Down Expand Up @@ -6301,6 +6325,22 @@
"category": "Message",
"code": 95159
},
"Add 'override' modifier": {
"category": "Message",
"code": 95160
},
"Remove 'override' modifier": {
"category": "Message",
"code": 95161
},
"Add all missing 'override' modifiers": {
"category": "Message",
"code": 95162
},
"Remove all unnecessary 'override' modifiers": {
"category": "Message",
"code": 95163
},

"No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
"category": "Error",
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/factory/nodeFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,8 @@ namespace ts {
get updateJSDocProtectedTag() { return getJSDocSimpleTagUpdateFunction<JSDocProtectedTag>(SyntaxKind.JSDocProtectedTag); },
get createJSDocReadonlyTag() { return getJSDocSimpleTagCreateFunction<JSDocReadonlyTag>(SyntaxKind.JSDocReadonlyTag); },
get updateJSDocReadonlyTag() { return getJSDocSimpleTagUpdateFunction<JSDocReadonlyTag>(SyntaxKind.JSDocReadonlyTag); },
get createJSDocOverrideTag() { return getJSDocSimpleTagCreateFunction<JSDocOverrideTag>(SyntaxKind.JSDocOverrideTag); },
get updateJSDocOverrideTag() { return getJSDocSimpleTagUpdateFunction<JSDocOverrideTag>(SyntaxKind.JSDocOverrideTag); },
get createJSDocDeprecatedTag() { return getJSDocSimpleTagCreateFunction<JSDocDeprecatedTag>(SyntaxKind.JSDocDeprecatedTag); },
get updateJSDocDeprecatedTag() { return getJSDocSimpleTagUpdateFunction<JSDocDeprecatedTag>(SyntaxKind.JSDocDeprecatedTag); },
createJSDocUnknownTag,
Expand Down Expand Up @@ -1041,6 +1043,7 @@ namespace ts {
if (flags & ModifierFlags.Static) { result.push(createModifier(SyntaxKind.StaticKeyword)); }
if (flags & ModifierFlags.Readonly) { result.push(createModifier(SyntaxKind.ReadonlyKeyword)); }
if (flags & ModifierFlags.Async) { result.push(createModifier(SyntaxKind.AsyncKeyword)); }
if (flags & ModifierFlags.Override) { result.push(createModifier(SyntaxKind.OverrideKeyword)); }
return result;
}

Expand Down Expand Up @@ -5934,6 +5937,7 @@ namespace ts {
case SyntaxKind.JSDocPrivateTag: return "private";
case SyntaxKind.JSDocProtectedTag: return "protected";
case SyntaxKind.JSDocReadonlyTag: return "readonly";
case SyntaxKind.JSDocOverrideTag: return "override";
case SyntaxKind.JSDocTemplateTag: return "template";
case SyntaxKind.JSDocTypedefTag: return "typedef";
case SyntaxKind.JSDocParameterTag: return "param";
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/factory/nodeTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,10 @@ namespace ts {
return node.kind === SyntaxKind.JSDocReadonlyTag;
}

export function isJSDocOverrideTag(node: Node): node is JSDocOverrideTag {
return node.kind === SyntaxKind.JSDocOverrideTag;
}

export function isJSDocDeprecatedTag(node: Node): node is JSDocDeprecatedTag {
return node.kind === SyntaxKind.JSDocDeprecatedTag;
}
Expand Down
3 changes: 3 additions & 0 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7542,6 +7542,9 @@ namespace ts {
case "readonly":
tag = parseSimpleTag(start, factory.createJSDocReadonlyTag, tagName, margin, indentText);
break;
case "override":
tag = parseSimpleTag(start, factory.createJSDocOverrideTag, tagName, margin, indentText);
break;
case "deprecated":
hasDeprecatedTag = true;
tag = parseSimpleTag(start, factory.createJSDocDeprecatedTag, tagName, margin, indentText);
Expand Down
1 change: 1 addition & 0 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2138,6 +2138,7 @@ namespace ts {
case SyntaxKind.ReadonlyKeyword:
case SyntaxKind.DeclareKeyword:
case SyntaxKind.AbstractKeyword:
case SyntaxKind.OverrideKeyword:
diagnostics.push(createDiagnosticForNode(modifier, Diagnostics.The_0_modifier_can_only_be_used_in_TypeScript_files, tokenToString(modifier.kind)));
break;

Expand Down
1 change: 1 addition & 0 deletions src/compiler/scanner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ namespace ts {
private: SyntaxKind.PrivateKeyword,
protected: SyntaxKind.ProtectedKeyword,
public: SyntaxKind.PublicKeyword,
override: SyntaxKind.OverrideKeyword,
readonly: SyntaxKind.ReadonlyKeyword,
require: SyntaxKind.RequireKeyword,
global: SyntaxKind.GlobalKeyword,
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/transformers/declarations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1540,7 +1540,7 @@ namespace ts {
}

function ensureModifierFlags(node: Node): ModifierFlags {
let mask = ModifierFlags.All ^ (ModifierFlags.Public | ModifierFlags.Async); // No async modifiers in declaration files
let mask = ModifierFlags.All ^ (ModifierFlags.Public | ModifierFlags.Async | ModifierFlags.Override); // No async and override modifiers in declaration files
let additions = (needsDeclare && !isAlwaysType(node)) ? ModifierFlags.Ambient : ModifierFlags.None;
const parentIsFile = node.parent.kind === SyntaxKind.SourceFile;
if (!parentIsFile || (isBundledEmit && parentIsFile && isExternalModule(node.parent as SourceFile))) {
Expand Down