Skip to content

Support for protected members in classes #688

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.

Sign up for GitHub

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 6 commits into from
Sep 19, 2014
Merged

Support for protected members in classes #688

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ce6b623
Support for protected members in classes
ahejlsberg Sep 17, 2014
4aa04a9
Addressing CR feedback
ahejlsberg Sep 17, 2014
08188b0
Merge branch 'master' into protectedMembers
ahejlsberg Sep 18, 2014
c990c42
Improved error messages for private and protected member access
ahejlsberg Sep 19, 2014
ddfe28d
Accepting new baselines
ahejlsberg Sep 19, 2014
b5b0777
More tests for protected members
ahejlsberg Sep 19, 2014
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
2 changes: 1 addition & 1 deletion src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ module ts {
function bindConstructorDeclaration(node: ConstructorDeclaration) {
bindDeclaration(node, SymbolFlags.Constructor, 0);
forEach(node.parameters, p => {
if (p.flags & (NodeFlags.Public | NodeFlags.Private)) {
if (p.flags & (NodeFlags.Public | NodeFlags.Private | NodeFlags.Protected)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I have recently added NodeFlags.AccessibilityModifier, a merge should bring it to your branch

bindDeclaration(p, SymbolFlags.Property, SymbolFlags.PropertyExcludes);
}
});
Expand Down
184 changes: 109 additions & 75 deletions src/compiler/checker.ts
View file
Open in desktop

Large diffs are not rendered by default.

11 changes: 7 additions & 4 deletions src/compiler/diagnosticInformationMap.generated.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,9 @@ module ts {
Type_0_is_not_assignable_to_type_1_Colon: { code: 2322, category: DiagnosticCategory.Error, key: "Type '{0}' is not assignable to type '{1}':" },
Type_0_is_not_assignable_to_type_1: { code: 2323, category: DiagnosticCategory.Error, key: "Type '{0}' is not assignable to type '{1}'." },
Property_0_is_missing_in_type_1: { code: 2324, category: DiagnosticCategory.Error, key: "Property '{0}' is missing in type '{1}'." },
Private_property_0_cannot_be_reimplemented: { code: 2325, category: DiagnosticCategory.Error, key: "Private property '{0}' cannot be reimplemented." },
Property_0_is_private_in_type_1_but_not_in_type_2: { code: 2325, category: DiagnosticCategory.Error, key: "Property '{0}' is private in type '{1}' but not in type '{2}'." },
Types_of_property_0_are_incompatible_Colon: { code: 2326, category: DiagnosticCategory.Error, key: "Types of property '{0}' are incompatible:" },
Required_property_0_cannot_be_reimplemented_with_optional_property_in_1: { code: 2327, category: DiagnosticCategory.Error, key: "Required property '{0}' cannot be reimplemented with optional property in '{1}'." },
Property_0_is_optional_in_type_1_but_required_in_type_2: { code: 2327, category: DiagnosticCategory.Error, key: "Property '{0}' is optional in type '{1}' but required in type '{2}'." },
Types_of_parameters_0_and_1_are_incompatible_Colon: { code: 2328, category: DiagnosticCategory.Error, key: "Types of parameters '{0}' and '{1}' are incompatible:" },
Index_signature_is_missing_in_type_0: { code: 2329, category: DiagnosticCategory.Error, key: "Index signature is missing in type '{0}'." },
Index_signatures_are_incompatible_Colon: { code: 2330, category: DiagnosticCategory.Error, key: "Index signatures are incompatible:" },
Expand All @@ -153,7 +153,7 @@ module ts {
Super_calls_are_not_permitted_outside_constructors_or_in_nested_functions_inside_constructors: { code: 2337, category: DiagnosticCategory.Error, key: "Super calls are not permitted outside constructors or in nested functions inside constructors" },
super_property_access_is_permitted_only_in_a_constructor_member_function_or_member_accessor_of_a_derived_class: { code: 2338, category: DiagnosticCategory.Error, key: "'super' property access is permitted only in a constructor, member function, or member accessor of a derived class" },
Property_0_does_not_exist_on_type_1: { code: 2339, category: DiagnosticCategory.Error, key: "Property '{0}' does not exist on type '{1}'." },
Only_public_methods_of_the_base_class_are_accessible_via_the_super_keyword: { code: 2340, category: DiagnosticCategory.Error, key: "Only public methods of the base class are accessible via the 'super' keyword" },
Only_public_and_protected_methods_of_the_base_class_are_accessible_via_the_super_keyword: { code: 2340, category: DiagnosticCategory.Error, key: "Only public and protected methods of the base class are accessible via the 'super' keyword" },
Property_0_is_inaccessible: { code: 2341, category: DiagnosticCategory.Error, key: "Property '{0}' is inaccessible." },
An_index_expression_argument_must_be_of_type_string_number_or_any: { code: 2342, category: DiagnosticCategory.Error, key: "An index expression argument must be of type 'string', 'number', or 'any'." },
Type_0_does_not_satisfy_the_constraint_1_Colon: { code: 2343, category: DiagnosticCategory.Error, key: "Type '{0}' does not satisfy the constraint '{1}':" },
Expand Down Expand Up @@ -198,7 +198,7 @@ module ts {
Specialized_overload_signature_is_not_assignable_to_any_non_specialized_signature: { code: 2382, category: DiagnosticCategory.Error, key: "Specialized overload signature is not assignable to any non-specialized signature." },
Overload_signatures_must_all_be_exported_or_not_exported: { code: 2383, category: DiagnosticCategory.Error, key: "Overload signatures must all be exported or not exported." },
Overload_signatures_must_all_be_ambient_or_non_ambient: { code: 2384, category: DiagnosticCategory.Error, key: "Overload signatures must all be ambient or non-ambient." },
Overload_signatures_must_all_be_public_or_private: { code: 2385, category: DiagnosticCategory.Error, key: "Overload signatures must all be public or private." },
Overload_signatures_must_all_be_public_private_or_protected: { code: 2385, category: DiagnosticCategory.Error, key: "Overload signatures must all be public, private or protected." },
Overload_signatures_must_all_be_optional_or_required: { code: 2386, category: DiagnosticCategory.Error, key: "Overload signatures must all be optional or required." },
Function_overload_must_be_static: { code: 2387, category: DiagnosticCategory.Error, key: "Function overload must be static." },
Function_overload_must_not_be_static: { code: 2388, category: DiagnosticCategory.Error, key: "Function overload must not be static." },
Expand Down Expand Up @@ -255,6 +255,9 @@ module ts {
Import_declaration_in_an_ambient_external_module_declaration_cannot_reference_external_module_through_relative_external_module_name: { code: 2439, category: DiagnosticCategory.Error, key: "Import declaration in an ambient external module declaration cannot reference external module through relative external module name." },
Import_declaration_conflicts_with_local_declaration_of_0: { code: 2440, category: DiagnosticCategory.Error, key: "Import declaration conflicts with local declaration of '{0}'" },
Duplicate_identifier_0_Compiler_reserves_name_1_in_top_level_scope_of_an_external_module: { code: 2441, category: DiagnosticCategory.Error, key: "Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of an external module." },
Types_have_separate_declarations_of_a_private_property_0: { code: 2442, category: DiagnosticCategory.Error, key: "Types have separate declarations of a private property '{0}'." },
Property_0_is_protected_but_type_1_is_not_derived_from_type_2: { code: 2443, category: DiagnosticCategory.Error, key: "Property '{0}' is protected but type '{1}' is not derived from type '{2}'." },
Property_0_is_protected_in_type_1_but_public_in_type_2: { code: 2444, category: DiagnosticCategory.Error, key: "Property '{0}' is protected in type '{1}' but public in type '{2}'." },
Import_declaration_0_is_using_private_name_1: { code: 4000, category: DiagnosticCategory.Error, key: "Import declaration '{0}' is using private name '{1}'." },
Type_parameter_0_of_exported_class_has_or_is_using_name_1_from_private_module_2: { code: 4001, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of exported class has or is using name '{1}' from private module '{2}'." },
Type_parameter_0_of_exported_class_has_or_is_using_private_name_1: { code: 4002, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of exported class has or is using private name '{1}'." },
Expand Down
21 changes: 16 additions & 5 deletions src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,15 +544,15 @@
"category": "Error",
"code": 2324
},
"Private property '{0}' cannot be reimplemented.": {
"Property '{0}' is private in type '{1}' but not in type '{2}'.": {
"category": "Error",
"code": 2325
},
"Types of property '{0}' are incompatible:": {
"category": "Error",
"code": 2326
},
"Required property '{0}' cannot be reimplemented with optional property in '{1}'.": {
"Property '{0}' is optional in type '{1}' but required in type '{2}'.": {
"category": "Error",
"code": 2327
},
Expand Down Expand Up @@ -604,7 +604,7 @@
"category": "Error",
"code": 2339
},
"Only public methods of the base class are accessible via the 'super' keyword": {
"Only public and protected methods of the base class are accessible via the 'super' keyword": {
"category": "Error",
"code": 2340
},
Expand Down Expand Up @@ -784,7 +784,7 @@
"category": "Error",
"code": 2384
},
"Overload signatures must all be public or private.": {
"Overload signatures must all be public, private or protected.": {
"category": "Error",
"code": 2385
},
Expand Down Expand Up @@ -1012,7 +1012,18 @@
"category": "Error",
"code": 2441
},

"Types have separate declarations of a private property '{0}'.": {
"category": "Error",
"code": 2442
},
"Property '{0}' is protected but type '{1}' is not derived from type '{2}'.": {
"category": "Error",
"code": 2443
},
"Property '{0}' is protected in type '{1}' but public in type '{2}'.": {
"category": "Error",
"code": 2444
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Make sure indentation is consistent here

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure what's going on, indentation is fine in the actual file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe try opening it in notepad? Visual studio might interpret tabs differently

Copy link
Member

Choose a reason for hiding this comment

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

Ensure there are absolutely no tabs here.


"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
Expand Down
10 changes: 8 additions & 2 deletions src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1431,7 +1431,7 @@ module ts {

function emitParameterPropertyAssignments(node: ConstructorDeclaration) {
forEach(node.parameters, param => {
if (param.flags & (NodeFlags.Public | NodeFlags.Private)) {
if (param.flags & (NodeFlags.Public | NodeFlags.Private | NodeFlags.Protected)) {
writeLine();
emitStart(param);
emitStart(param.name);
Expand Down Expand Up @@ -2368,12 +2368,18 @@ module ts {
if (node.flags & NodeFlags.Private) {
Copy link
Contributor

Choose a reason for hiding this comment

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

these seem repeated, I would move them before the if statement

Copy link
Contributor

Choose a reason for hiding this comment

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

never mind.

write("private ");
}
else if (node.flags & NodeFlags.Protected) {
write("protected ");
}
write("static ");
}
else {
if (node.flags & NodeFlags.Private) {
write("private ");
}
else if (node.flags & NodeFlags.Protected) {
write("protected ");
}
// If the node is parented in the current source file we need to emit export declare or just export
else if (node.parent === currentSourceFile) {
// If the node is exported
Expand Down Expand Up @@ -2630,7 +2636,7 @@ module ts {
function emitParameterProperties(constructorDeclaration: ConstructorDeclaration) {
if (constructorDeclaration) {
forEach(constructorDeclaration.parameters, param => {
if (param.flags & (NodeFlags.Public | NodeFlags.Private)) {
if (param.flags & (NodeFlags.Public | NodeFlags.Private | NodeFlags.Protected)) {
emitPropertyDeclaration(param);
}
});
Expand Down
27 changes: 25 additions & 2 deletions src/compiler/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,7 @@ module ts {
switch (token) {
case SyntaxKind.PublicKeyword:
case SyntaxKind.PrivateKeyword:
case SyntaxKind.ProtectedKeyword:
case SyntaxKind.StaticKeyword:
case SyntaxKind.ExportKeyword:
case SyntaxKind.DeclareKeyword:
Expand Down Expand Up @@ -2883,6 +2884,7 @@ module ts {
}
case SyntaxKind.PublicKeyword:
case SyntaxKind.PrivateKeyword:
case SyntaxKind.ProtectedKeyword:
case SyntaxKind.StaticKeyword:
// When followed by an identifier or keyword, these do not start a statement but
// might instead be following type members
Expand Down Expand Up @@ -3201,6 +3203,8 @@ module ts {
var lastDeclareModifierLength: number;
var lastPrivateModifierStart: number;
var lastPrivateModifierLength: number;
var lastProtectedModifierStart: number;
var lastProtectedModifierLength: number;

while (true) {
var modifierStart = scanner.getTokenPos();
Expand All @@ -3213,7 +3217,7 @@ module ts {

switch (modifierToken) {
case SyntaxKind.PublicKeyword:
if (flags & NodeFlags.Private || flags & NodeFlags.Public) {
if (flags & NodeFlags.Public || flags & NodeFlags.Private || flags & NodeFlags.Protected) {
grammarErrorAtPos(modifierStart, modifierLength, Diagnostics.Accessibility_modifier_already_seen);
}
else if (flags & NodeFlags.Static) {
Expand All @@ -3226,7 +3230,7 @@ module ts {
break;

case SyntaxKind.PrivateKeyword:
if (flags & NodeFlags.Private || flags & NodeFlags.Public) {
if (flags & NodeFlags.Public || flags & NodeFlags.Private || flags & NodeFlags.Protected) {
grammarErrorAtPos(modifierStart, modifierLength, Diagnostics.Accessibility_modifier_already_seen);
}
else if (flags & NodeFlags.Static) {
Expand All @@ -3240,6 +3244,21 @@ module ts {
flags |= NodeFlags.Private;
break;

case SyntaxKind.ProtectedKeyword:
if (flags & NodeFlags.Public || flags & NodeFlags.Private || flags & NodeFlags.Protected) {
grammarErrorAtPos(modifierStart, modifierLength, Diagnostics.Accessibility_modifier_already_seen);
}
else if (flags & NodeFlags.Static) {
grammarErrorAtPos(modifierStart, modifierLength, Diagnostics._0_modifier_must_precede_1_modifier, "protected", "static");
}
else if (context === ModifierContext.ModuleElements || context === ModifierContext.SourceElements) {
grammarErrorAtPos(modifierStart, modifierLength, Diagnostics._0_modifier_cannot_appear_on_a_module_element, "protected");
}
lastProtectedModifierStart = modifierStart;
lastProtectedModifierLength = modifierLength;
flags |= NodeFlags.Protected;
break;

case SyntaxKind.StaticKeyword:
if (flags & NodeFlags.Static) {
grammarErrorAtPos(modifierStart, modifierLength, Diagnostics._0_modifier_already_seen, "static");
Expand Down Expand Up @@ -3297,6 +3316,9 @@ module ts {
else if (token === SyntaxKind.ConstructorKeyword && flags & NodeFlags.Private) {
grammarErrorAtPos(lastPrivateModifierStart, lastPrivateModifierLength, Diagnostics._0_modifier_cannot_appear_on_a_constructor_declaration, "private");
}
else if (token === SyntaxKind.ConstructorKeyword && flags & NodeFlags.Protected) {
grammarErrorAtPos(lastProtectedModifierStart, lastProtectedModifierLength, Diagnostics._0_modifier_cannot_appear_on_a_constructor_declaration, "protected");
}
else if (token === SyntaxKind.ImportKeyword) {
if (flags & NodeFlags.Ambient) {
grammarErrorAtPos(lastDeclareModifierStart, lastDeclareModifierLength, Diagnostics.A_declare_modifier_cannot_be_used_with_an_import_declaration, "declare");
Expand Down Expand Up @@ -3573,6 +3595,7 @@ module ts {
case SyntaxKind.DeclareKeyword:
case SyntaxKind.PublicKeyword:
case SyntaxKind.PrivateKeyword:
case SyntaxKind.ProtectedKeyword:
case SyntaxKind.StaticKeyword:
// Check for modifier on source element
return lookAhead(() => { nextToken(); return isDeclaration(); });
Expand Down
11 changes: 6 additions & 5 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,12 +232,13 @@ module ts {
Rest = 0x00000008, // Parameter
Public = 0x00000010, // Property/Method
Private = 0x00000020, // Property/Method
Static = 0x00000040, // Property/Method
MultiLine = 0x00000080, // Multi-line array or object literal
Synthetic = 0x00000100, // Synthetic node (for full fidelity)
DeclarationFile = 0x00000200, // Node is a .d.ts file
Protected = 0x00000040, // Property/Method
Static = 0x00000080, // Property/Method
MultiLine = 0x00000100, // Multi-line array or object literal
Synthetic = 0x00000200, // Synthetic node (for full fidelity)
DeclarationFile = 0x00000400, // Node is a .d.ts file

Modifier = Export | Ambient | Public | Private | Static
Modifier = Export | Ambient | Public | Private | Protected | Static
Copy link
Contributor

Choose a reason for hiding this comment

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

I think @mhegazy added a flag for accessibility modifiers. Can you check with him, and add Protected to that set too?

}

export interface Node extends TextRange {
Expand Down
Loading