Skip to content

Commit

Permalink
Report privacy errors on return types
Browse files Browse the repository at this point in the history
  • Loading branch information
sheetalkamat committed Aug 7, 2014
1 parent 4115077 commit 4cd2d3f
Show file tree
Hide file tree
Showing 13 changed files with 5,036 additions and 51 deletions.
14 changes: 14 additions & 0 deletions src/compiler/diagnosticInformationMap.generated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,20 @@ module ts {
Parameter_0_of_public_method_from_exported_class_has_or_is_using_name_1_from_private_module_2: { code: 2047, category: DiagnosticCategory.Error, key: "Parameter '{0}' of public method from exported class has or is using name '{1}' from private module '{2}'." },
Parameter_0_of_method_from_exported_interface_has_or_is_using_name_1_from_private_module_2: { code: 2048, category: DiagnosticCategory.Error, key: "Parameter '{0}' of method from exported interface has or is using name '{1}' from private module '{2}'." },
Parameter_0_of_exported_function_has_or_is_using_name_1_from_private_module_2: { code: 2049, category: DiagnosticCategory.Error, key: "Parameter '{0}' of exported function has or is using name '{1}' from private module '{2}'." },
Return_type_of_constructor_signature_from_exported_interface_has_or_is_using_private_name_0: { code: 2052, category: DiagnosticCategory.Error, key: "Return type of constructor signature from exported interface has or is using private name '{0}'." },
Return_type_of_call_signature_from_exported_interface_has_or_is_using_private_name_0: { code: 2053, category: DiagnosticCategory.Error, key: "Return type of call signature from exported interface has or is using private name '{0}'." },
Return_type_of_index_signature_from_exported_interface_has_or_is_using_private_name_0: { code: 2054, category: DiagnosticCategory.Error, key: "Return type of index signature from exported interface has or is using private name '{0}'." },
Return_type_of_public_static_method_from_exported_class_has_or_is_using_private_name_0: { code: 2055, category: DiagnosticCategory.Error, key: "Return type of public static method from exported class has or is using private name '{0}'." },
Return_type_of_public_method_from_exported_class_has_or_is_using_private_name_0: { code: 2056, category: DiagnosticCategory.Error, key: "Return type of public method from exported class has or is using private name '{0}'." },
Return_type_of_method_from_exported_interface_has_or_is_using_private_name_0: { code: 2057, category: DiagnosticCategory.Error, key: "Return type of method from exported interface has or is using private name '{0}'." },
Return_type_of_exported_function_has_or_is_using_private_name_0: { code: 2058, category: DiagnosticCategory.Error, key: "Return type of exported function has or is using private name '{0}'." },
Return_type_of_constructor_signature_from_exported_interface_has_or_is_using_name_0_from_private_module_1: { code: 2061, category: DiagnosticCategory.Error, key: "Return type of constructor signature from exported interface has or is using name '{0}' from private module '{1}'." },
Return_type_of_call_signature_from_exported_interface_has_or_is_using_name_0_from_private_module_1: { code: 2062, category: DiagnosticCategory.Error, key: "Return type of call signature from exported interface has or is using name '{0}' from private module '{1}'." },
Return_type_of_index_signature_from_exported_interface_has_or_is_using_name_0_from_private_module_1: { code: 2063, category: DiagnosticCategory.Error, key: "Return type of index signature from exported interface has or is using name '{0}' from private module '{1}'." },
Return_type_of_public_static_method_from_exported_class_has_or_is_using_name_0_from_private_module_1: { code: 2064, category: DiagnosticCategory.Error, key: "Return type of public static method from exported class has or is using name '{0}' from private module '{1}'." },
Return_type_of_public_method_from_exported_class_has_or_is_using_name_0_from_private_module_1: { code: 2065, category: DiagnosticCategory.Error, key: "Return type of public method from exported class has or is using name '{0}' from private module '{1}'." },
Return_type_of_method_from_exported_interface_has_or_is_using_name_0_from_private_module_1: { code: 2066, category: DiagnosticCategory.Error, key: "Return type of method from exported interface has or is using name '{0}' from private module '{1}'." },
Return_type_of_exported_function_has_or_is_using_name_0_from_private_module_1: { code: 2067, category: DiagnosticCategory.Error, key: "Return type of exported function has or is using name '{0}' from private module '{1}'." },
Type_parameter_0_of_constructor_signature_from_exported_interface_has_or_is_using_private_name_1: { code: 2208, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of constructor signature from exported interface has or is using private name '{1}'." },
Type_parameter_0_of_call_signature_from_exported_interface_has_or_is_using_private_name_1: { code: 2209, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of call signature from exported interface has or is using private name '{1}'." },
Type_parameter_0_of_public_static_method_from_exported_class_has_or_is_using_private_name_1: { code: 2210, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of public static method from exported class has or is using private name '{1}'." },
Expand Down
56 changes: 56 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,62 @@
"category": "Error",
"code": 2049
},
"Return type of constructor signature from exported interface has or is using private name '{0}'.": {
"category": "Error",
"code": 2052
},
"Return type of call signature from exported interface has or is using private name '{0}'.": {
"category": "Error",
"code": 2053
},
"Return type of index signature from exported interface has or is using private name '{0}'.": {
"category": "Error",
"code": 2054
},
"Return type of public static method from exported class has or is using private name '{0}'.": {
"category": "Error",
"code": 2055
},
"Return type of public method from exported class has or is using private name '{0}'.": {
"category": "Error",
"code": 2056
},
"Return type of method from exported interface has or is using private name '{0}'.": {
"category": "Error",
"code": 2057
},
"Return type of exported function has or is using private name '{0}'.": {
"category": "Error",
"code": 2058
},
"Return type of constructor signature from exported interface has or is using name '{0}' from private module '{1}'.": {
"category": "Error",
"code": 2061
},
"Return type of call signature from exported interface has or is using name '{0}' from private module '{1}'.": {
"category": "Error",
"code": 2062
},
"Return type of index signature from exported interface has or is using name '{0}' from private module '{1}'.": {
"category": "Error",
"code": 2063
},
"Return type of public static method from exported class has or is using name '{0}' from private module '{1}'.": {
"category": "Error",
"code": 2064
},
"Return type of public method from exported class has or is using name '{0}' from private module '{1}'.": {
"category": "Error",
"code": 2065
},
"Return type of method from exported interface has or is using name '{0}' from private module '{1}'.": {
"category": "Error",
"code": 2066
},
"Return type of exported function has or is using name '{0}' from private module '{1}'.": {

This comment has been minimized.

Copy link
@mihailik

mihailik Aug 7, 2014

Contributor

Wouldn't it be clearer if it was specific: if it has name then it says 'has' if is using then it says 'using'?

No need to ask a human to do the job of the machine, right.

This comment has been minimized.

Copy link
@sheetalkamat

sheetalkamat Aug 7, 2014

Author Member

This is generic because it could be error from some object type which has say call signature that has parameter that uses the name
eg

module m {
   module privateModule {
      export class c {
      }
   } 
    export var x: (a: privateModule.c) => void;
}
"category": "Error",
"code": 2067
},
"Type parameter '{0}' of constructor signature from exported interface has or is using private name '{1}'.": {
"category": "Error",
"code": 2208
Expand Down
80 changes: 74 additions & 6 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1879,7 +1879,7 @@ module ts {
var getSymbolVisibilityDiagnosticMessage: (symbolAccesibilityResult: SymbolAccessiblityResult) => {
errorNode: Node;
diagnosticMessage: DiagnosticMessage;
typeName: Identifier
typeName?: Identifier
}

This comment has been minimized.

Copy link
@mihailik

mihailik Aug 7, 2014

Contributor

Missing semicolon?

This comment has been minimized.

Copy link
@sheetalkamat

sheetalkamat Aug 7, 2014

Author Member

thanks. will fix this.


function writeSymbol(symbol: Symbol, enclosingDeclaration?: Node, meaning?: SymbolFlags) {
Expand Down Expand Up @@ -1909,11 +1909,19 @@ module ts {
reportedDeclarationError = true;
var errorInfo = getSymbolVisibilityDiagnosticMessage(symbolAccesibilityResult);
if (errorInfo) {
diagnostics.push(createDiagnosticForNode(errorInfo.errorNode,
errorInfo.diagnosticMessage,
getSourceTextOfLocalNode(errorInfo.typeName),
symbolAccesibilityResult.errorSymbolName,
symbolAccesibilityResult.errorModuleName));
if (errorInfo.typeName) {
diagnostics.push(createDiagnosticForNode(errorInfo.errorNode,
errorInfo.diagnosticMessage,
getSourceTextOfLocalNode(errorInfo.typeName),
symbolAccesibilityResult.errorSymbolName,
symbolAccesibilityResult.errorModuleName));
}
else {
diagnostics.push(createDiagnosticForNode(errorInfo.errorNode,
errorInfo.diagnosticMessage,
symbolAccesibilityResult.errorSymbolName,
symbolAccesibilityResult.errorModuleName));
}
}
}
}
Expand Down Expand Up @@ -2394,10 +2402,70 @@ module ts {
// If this is not a constructor and is not private, emit the return type
if (node.kind !== SyntaxKind.Constructor && !(node.flags & NodeFlags.Private)) {
write(": ");
getSymbolVisibilityDiagnosticMessage = getReturnTypeVisibilityError;
resolver.writeReturnTypeOfSignatureDeclaration(node, enclosingDeclaration, TypeFormatFlags.None, writer);
// TODO(shkamat) This is just till we get rest of the error reporting up
getSymbolVisibilityDiagnosticMessage = undefined;
}
write(";");
writeLine();

function getReturnTypeVisibilityError(symbolAccesibilityResult: SymbolAccessiblityResult) {
// TODO(shkamat) Cannot access name errors
var diagnosticMessage: DiagnosticMessage;
switch (node.kind) {
case SyntaxKind.ConstructSignature:
diagnosticMessage = symbolAccesibilityResult.errorModuleName ?
Diagnostics.Return_type_of_constructor_signature_from_exported_interface_has_or_is_using_name_0_from_private_module_1 :
Diagnostics.Return_type_of_constructor_signature_from_exported_interface_has_or_is_using_private_name_0;
break;

case SyntaxKind.CallSignature:
diagnosticMessage = symbolAccesibilityResult.errorModuleName ?
Diagnostics.Return_type_of_call_signature_from_exported_interface_has_or_is_using_name_0_from_private_module_1 :
Diagnostics.Return_type_of_call_signature_from_exported_interface_has_or_is_using_private_name_0;
break;

case SyntaxKind.IndexSignature:
diagnosticMessage = symbolAccesibilityResult.errorModuleName ?
Diagnostics.Return_type_of_index_signature_from_exported_interface_has_or_is_using_name_0_from_private_module_1 :
Diagnostics.Return_type_of_index_signature_from_exported_interface_has_or_is_using_private_name_0;
break;

case SyntaxKind.Method:
if (node.flags & NodeFlags.Static) {
diagnosticMessage = symbolAccesibilityResult.errorModuleName ?
Diagnostics.Return_type_of_public_static_method_from_exported_class_has_or_is_using_name_0_from_private_module_1 :
Diagnostics.Return_type_of_public_static_method_from_exported_class_has_or_is_using_private_name_0;
}
else if (node.parent.kind === SyntaxKind.ClassDeclaration) {
diagnosticMessage = symbolAccesibilityResult.errorModuleName ?
Diagnostics.Return_type_of_public_method_from_exported_class_has_or_is_using_name_0_from_private_module_1 :
Diagnostics.Return_type_of_public_method_from_exported_class_has_or_is_using_private_name_0;
}
else {
diagnosticMessage = symbolAccesibilityResult.errorModuleName ?
Diagnostics.Return_type_of_method_from_exported_interface_has_or_is_using_name_0_from_private_module_1 :
Diagnostics.Return_type_of_method_from_exported_interface_has_or_is_using_private_name_0;
}
break;

case SyntaxKind.FunctionDeclaration:
diagnosticMessage = symbolAccesibilityResult.errorModuleName ?
Diagnostics.Return_type_of_exported_function_has_or_is_using_name_0_from_private_module_1 :
Diagnostics.Return_type_of_exported_function_has_or_is_using_private_name_0;
break;

default:
Debug.fail("This is unknown kind for signature: " + SyntaxKind[node.kind]);
}

return {
diagnosticMessage: diagnosticMessage,
errorNode: <Node>node.name || node,
};
}

}

function emitParameterDeclaration(node: ParameterDeclaration) {
Expand Down
6 changes: 5 additions & 1 deletion tests/baselines/reference/declInput-2.errors.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
==== tests/cases/compiler/declInput-2.ts (3 errors) ====
==== tests/cases/compiler/declInput-2.ts (5 errors) ====
module M {
class C { }
export class E {}
Expand All @@ -19,10 +19,14 @@
public m232(): E { return null;}
public m242(): I1 { return null; }
public m252(): I2 { return null; } // don't generate
~~~~
!!! Return type of public method from exported class has or is using private name 'I2'.
public m26(i:I1) {}
public m262(i:I2) {}
~~~~
!!! Parameter 'i' of public method from exported class has or is using private name 'I2'.
public m3():C { return new C(); }
~~
!!! Return type of public method from exported class has or is using private name 'C'.
}
}
16 changes: 16 additions & 0 deletions tests/baselines/reference/exportImport.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
==== tests/cases/compiler/consumer.ts (1 errors) ====
import e = require('./exporter');

export function w(): e.w { // Should be OK
~
!!! Return type of exported function has or is using private name 'Widget1'.
return new e.w();
}
==== tests/cases/compiler/w1.ts (0 errors) ====

export = Widget1
class Widget1 { name = 'one'; }

==== tests/cases/compiler/exporter.ts (0 errors) ====
export import w = require('./w1');

21 changes: 0 additions & 21 deletions tests/baselines/reference/exportImport.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,24 +45,3 @@ declare class Widget1 {
}
//// [exporter.d.ts]
export import w = require('./w1');
//// [consumer.d.ts]
export declare function w(): Widget1;


//// [DtsFileErrors]


==== tests/cases/compiler/consumer.d.ts (1 errors) ====
export declare function w(): Widget1;
~~~~~~~
!!! Cannot find name 'Widget1'.

==== tests/cases/compiler/w1.d.ts (0 errors) ====
export = Widget1;
declare class Widget1 {
name: string;
}

==== tests/cases/compiler/exporter.d.ts (0 errors) ====
export import w = require('./w1');

Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
==== tests/cases/compiler/consumer.ts (1 errors) ====
import e = require('./exporter');

export function w(): e.w { // Should be OK
~
!!! Return type of exported function has or is using private name 'Widget1'.
return {name: 'value' };
}
==== tests/cases/compiler/w1.ts (0 errors) ====

export = Widget1
interface Widget1 { name: string; }

==== tests/cases/compiler/exporter.ts (0 errors) ====
export import w = require('./w1');

21 changes: 0 additions & 21 deletions tests/baselines/reference/exportImportNonInstantiatedModule2.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,24 +37,3 @@ interface Widget1 {
}
//// [exporter.d.ts]
export import w = require('./w1');
//// [consumer.d.ts]
export declare function w(): Widget1;


//// [DtsFileErrors]


==== tests/cases/compiler/consumer.d.ts (1 errors) ====
export declare function w(): Widget1;
~~~~~~~
!!! Cannot find name 'Widget1'.

==== tests/cases/compiler/w1.d.ts (0 errors) ====
export = Widget1;
interface Widget1 {
name: string;
}

==== tests/cases/compiler/exporter.d.ts (0 errors) ====
export import w = require('./w1');

Loading

0 comments on commit 4cd2d3f

Please sign in to comment.