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

Don't include snippets of source in diagnostics about types #7515

Closed
wants to merge 12 commits into from
Closed
Show file tree
Hide file tree
Changes from 10 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
10 changes: 5 additions & 5 deletions src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1282,7 +1282,7 @@ internal Symbol ResultSymbol(
// '{0}' is an ambiguous reference between '{1}' and '{2}'
info = new CSDiagnosticInfo(ErrorCode.ERR_AmbigContext, originalSymbols,
new object[] {
where,
(where as NameSyntax)?.ErrorDisplayName() ?? simpleName,
new FormattedSymbol(first, SymbolDisplayFormat.CSharpErrorMessageFormat),
new FormattedSymbol(second, SymbolDisplayFormat.CSharpErrorMessageFormat) });
}
Expand Down Expand Up @@ -1421,14 +1421,14 @@ internal Symbol ResultSymbol(
// SPEC: is present, and a compile-time error results.

info = new CSDiagnosticInfo(ErrorCode.ERR_AmbiguousAttribute, originalSymbols,
new object[] { where, first, second });
new object[] { (where as NameSyntax)?.ErrorDisplayName() ?? simpleName, first, second });
}
else
{
// '{0}' is an ambiguous reference between '{1}' and '{2}'
info = new CSDiagnosticInfo(ErrorCode.ERR_AmbigContext, originalSymbols,
new object[] {
where,
(where as NameSyntax)?.ErrorDisplayName() ?? simpleName,
new FormattedSymbol(first, SymbolDisplayFormat.CSharpErrorMessageFormat),
new FormattedSymbol(second, SymbolDisplayFormat.CSharpErrorMessageFormat) });
}
Expand Down Expand Up @@ -1524,7 +1524,7 @@ internal Symbol ResultSymbol(
node = node.Parent;
}

CSDiagnosticInfo info = NotFound(where, simpleName, arity, where.ToString(), diagnostics, aliasOpt, qualifierOpt, options);
CSDiagnosticInfo info = NotFound(where, simpleName, arity, (where as NameSyntax)?.ErrorDisplayName() ?? simpleName, diagnostics, aliasOpt, qualifierOpt, options);
return new ExtendedErrorTypeSymbol(qualifierOpt ?? Compilation.Assembly.GlobalNamespace, simpleName, arity, info);
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like lines 1285, 1424, 1431 are also using syntax node as a diagnostics argument. Should they be changed too?

Copy link
Member Author

Choose a reason for hiding this comment

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

There are no compiler error codes between 1113 and 1501, so I'm not sure what you're referring to. From ErrorCode.cs:

        ERR_ExplicitExtension = 1112,
        ERR_ValueTypeExtDelegate = 1113,
        // unused 1114-1199
        // Below five error codes are unused.
        // WRN_FeatureDeprecated2 = 1200,
        // WRN_FeatureDeprecated3 = 1201,
        // WRN_FeatureDeprecated4 = 1202,
        // WRN_FeatureDeprecated5 = 1203,
        // WRN_OldWarning_FeatureDefaultDeprecated = 1204,
        // unused 1205-1500
        ERR_BadArgCount = 1501,
        //ERR_BadArgTypes = 1502,
        ERR_BadArgType = 1503,
        ERR_NoSourceFile = 1504,

Copy link
Contributor

Choose a reason for hiding this comment

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

I am referring to lines of code in this file.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like lines of code 1285, 1424, 1431 are also using syntax node as a diagnostics argument. Should they be changed too?

Copy link
Member Author

Choose a reason for hiding this comment

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

@AlekseyTs Those are fixed now, too. Can you please re-review?

}

Expand Down Expand Up @@ -1819,7 +1819,7 @@ private CSDiagnosticInfo NotFound(CSharpSyntaxNode where, string simpleName, int
return diagnostics.Add(ErrorCode.ERR_AliasNotFound, location, whereText);
}

if (whereText == "var" && !options.IsAttributeTypeLookup())
if ((where as IdentifierNameSyntax)?.Identifier.Text == "var" && !options.IsAttributeTypeLookup())
{
var code = (where.Parent is QueryClauseSyntax) ? ErrorCode.ERR_TypeVarNotFoundRangeVariable : ErrorCode.ERR_TypeVarNotFound;
return diagnostics.Add(code, location);
Expand Down
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/CSharpCodeAnalysis.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -750,6 +750,7 @@
<Compile Include="Syntax\GenericNameSyntax.cs" />
<Compile Include="Syntax\InternalSyntax\BinaryExpressionSyntax.cs" />
<Compile Include="Syntax\CSharpSyntaxTree.DebuggerSyntaxTree.cs" />
<Compile Include="Syntax\IdentifierNameSyntax.cs" />
<Compile Include="Syntax\PropertyDeclarationSyntax.cs" />
<Compile Include="Syntax\InternalSyntax\ChildSyntaxList.cs" />
<Compile Include="Syntax\InternalSyntax\ChildSyntaxList.Enumerator.cs" />
Expand Down Expand Up @@ -901,4 +902,4 @@
<ImportGroup Label="Targets">
<Import Project="..\..\..\..\build\Targets\VSL.Imports.targets" />
</ImportGroup>
</Project>
</Project>
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Symbols/Symbol_Attributes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,7 @@ private bool ValidateAttributeUsage(
// Given attribute can't be specified more than once if AllowMultiple is false.
if (!uniqueAttributeTypes.Add(attributeType) && !attributeUsageInfo.AllowMultiple)
{
diagnostics.Add(ErrorCode.ERR_DuplicateAttribute, node.Name.Location, node.Name);
diagnostics.Add(ErrorCode.ERR_DuplicateAttribute, node.Name.Location, node.GetErrorDisplayName());
return false;
}

Expand All @@ -675,7 +675,7 @@ private bool ValidateAttributeUsage(
if ((attributeTarget & attributeUsageInfo.ValidTargets) == 0)
{
// generate error
diagnostics.Add(ErrorCode.ERR_AttributeOnBadSymbolType, node.Name.Location, node.Name, attributeUsageInfo.GetValidTargetsErrorArgument());
diagnostics.Add(ErrorCode.ERR_AttributeOnBadSymbolType, node.Name.Location, node.GetErrorDisplayName(), attributeUsageInfo.GetValidTargetsErrorArgument());
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,10 @@ internal override SimpleNameSyntax GetUnqualifiedName()
{
return this.Name;
}

internal override string ErrorDisplayName()
{
return Alias.ErrorDisplayName() + "::" + Name.ErrorDisplayName();
}
}
}
5 changes: 1 addition & 4 deletions src/Compilers/CSharp/Portable/Syntax/AttributeSyntax.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@

using System;
using System.Diagnostics;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Text;

namespace Microsoft.CodeAnalysis.CSharp.Syntax
{
Expand All @@ -18,7 +15,7 @@ public partial class AttributeSyntax
internal string GetErrorDisplayName()
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be simply an override of ErrorDisplayName?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess not, because this type doesn't derive from NameSyntax.

{
// Dev10 uses the name from source, even if it's an alias.
return Name.ToString();
return Name.ErrorDisplayName();
}

internal AttributeArgumentSyntax GetNamedArgumentSyntax(string namedArgName)
Expand Down
8 changes: 8 additions & 0 deletions src/Compilers/CSharp/Portable/Syntax/GenericNameSyntax.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using Microsoft.CodeAnalysis.Collections;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Text;
Expand All @@ -15,5 +16,12 @@ public bool IsUnboundGenericName
return this.TypeArgumentList.Arguments.Any(SyntaxKind.OmittedTypeArgument);
}
}

internal override string ErrorDisplayName()
{
var pb = PooledStringBuilder.GetInstance();
pb.Builder.Append(Identifier.ValueText).Append("<").Append(',', Arity - 1).Append(">");
return pb.ToStringAndFree();
}
Copy link
Member

Choose a reason for hiding this comment

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

Are we testing these cases?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, some of the changed tests are due to this.

}
}
12 changes: 12 additions & 0 deletions src/Compilers/CSharp/Portable/Syntax/IdentifierNameSyntax.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace Microsoft.CodeAnalysis.CSharp.Syntax
{
public partial class IdentifierNameSyntax
{
internal override string ErrorDisplayName()
{
return Identifier.ValueText;
}
}
}
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Syntax/NameSyntax.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ public int Arity
/// </returns>
internal abstract SimpleNameSyntax GetUnqualifiedName();

/// <summary>
/// Return the name in string form, without trivia or generic arguments, for use in diagnostics.
/// </summary>
internal abstract string ErrorDisplayName();

/// <remarks>
/// This inspection is entirely syntactic. We are not trying to find the alias corresponding to the assembly symbol
/// containing the explicitly implemented interface symbol - there may be more than one. We just want to know
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Syntax/QualifiedNameSyntax.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,10 @@ internal override SimpleNameSyntax GetUnqualifiedName()
{
return Right;
}

internal override string ErrorDisplayName()
{
return Left.ErrorDisplayName() + "." + Right.ErrorDisplayName();
Copy link
Member

Choose a reason for hiding this comment

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

Are we testing this case and the AliasedQualifiedNameSyntax case?

Copy link
Member Author

Choose a reason for hiding this comment

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

Some of the test changes are due to this.

}
}
}
18 changes: 11 additions & 7 deletions src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3596,7 +3596,7 @@ public void AttributeNoMultipleAndInvalidTarget()
string source = @"
using CustomAttribute;
[Base(1)]
[BaseAttribute(""SOS"")]
[@BaseAttribute(""SOS"")]
static class AttributeMod
{
[Derived('Q')]
Expand All @@ -3617,13 +3617,17 @@ public class Bar

compilation.VerifyDiagnostics(
// (4,2): error CS0579: Duplicate 'BaseAttribute' attribute
Diagnostic(ErrorCode.ERR_DuplicateAttribute, @"BaseAttribute").WithArguments("BaseAttribute"),
// [@BaseAttribute("SOS")]
Diagnostic(ErrorCode.ERR_DuplicateAttribute, "@BaseAttribute").WithArguments("BaseAttribute").WithLocation(4, 2),
// (7,6): error CS0592: Attribute 'Derived' is not valid on this declaration type. It is only valid on 'struct, method, parameter' declarations.
Diagnostic(ErrorCode.ERR_AttributeOnBadSymbolType, "Derived").WithArguments("Derived", "struct, method, parameter"),
// [Derived('Q')]
Diagnostic(ErrorCode.ERR_AttributeOnBadSymbolType, "Derived").WithArguments("Derived", "struct, method, parameter").WithLocation(7, 6),
// (8,6): error CS0579: Duplicate 'Derived' attribute
Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Derived").WithArguments("Derived"),
// [Derived('C')]
Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Derived").WithArguments("Derived").WithLocation(8, 6),
// (13,6): error CS0579: Duplicate 'Base' attribute
Diagnostic(ErrorCode.ERR_DuplicateAttribute, @"Base").WithArguments("Base"));
// [Base("")]
Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Base").WithArguments("Base").WithLocation(13, 6));
}

[Fact]
Expand Down Expand Up @@ -3681,9 +3685,9 @@ class Class3 { }
var compilation = CreateCompilationWithMscorlib(source);

compilation.VerifyDiagnostics(
// (13,2): error CS0246: The type or namespace name '@X' could not be found (are you missing a using directive or an assembly reference?)
// (13,2): error CS0246: The type or namespace name 'X' could not be found (are you missing a using directive or an assembly reference?)
// [@X] // Error: No attribute named X
Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "@X").WithArguments("@X").WithLocation(13, 2));
Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "@X").WithArguments("X").WithLocation(13, 2));
}

[Fact]
Expand Down
25 changes: 13 additions & 12 deletions src/Compilers/CSharp/Test/Emit/PDB/PDBUsingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2154,33 +2154,34 @@ static void Main()
var comp = CreateCompilationWithMscorlib(source);

comp.VerifyDiagnostics(
// (6,11): error CS0246: The type or namespace name 'F<int>' could not be found (are you missing a using directive or an assembly reference?)
// (6,11): error CS0246: The type or namespace name 'F<>' could not be found (are you missing a using directive or an assembly reference?)
// using Z = F<int>;
Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "F<int>").WithArguments("F<int>"),
Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "F<int>").WithArguments("F<>").WithLocation(6, 11),
// (5,14): error CS0234: The type or namespace name 'E' does not exist in the namespace 'A' (are you missing an assembly reference?)
// using Y = A::E;
Diagnostic(ErrorCode.ERR_DottedTypeNameNotFoundInNS, "E").WithArguments("E", "A"),
Diagnostic(ErrorCode.ERR_DottedTypeNameNotFoundInNS, "E").WithArguments("E", "A").WithLocation(5, 14),
// (4,13): error CS0426: The type name 'D' does not exist in the type 'C'
// using X = C.D;
Diagnostic(ErrorCode.ERR_DottedTypeNameNotFoundInAgg, "D").WithArguments("D", "C"),
Diagnostic(ErrorCode.ERR_DottedTypeNameNotFoundInAgg, "D").WithArguments("D", "C").WithLocation(4, 13),
// (2,14): error CS0430: The extern alias 'A' was not specified in a /reference option
// extern alias A;
Diagnostic(ErrorCode.ERR_BadExternAlias, "A").WithArguments("A"),
Diagnostic(ErrorCode.ERR_BadExternAlias, "A").WithArguments("A").WithLocation(2, 14),
// (3,7): error CS0246: The type or namespace name 'B' could not be found (are you missing a using directive or an assembly reference?)
// using B;
Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "B").WithArguments("B"),
Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "B").WithArguments("B").WithLocation(3, 7),
// (5,1): hidden CS8019: Unnecessary using directive.
// using Y = A::E;
Diagnostic(ErrorCode.HDN_UnusedUsingDirective, "using Y = A::E;").WithLocation(5, 1),
// (3,1): hidden CS8019: Unnecessary using directive.
// using B;
Diagnostic(ErrorCode.HDN_UnusedUsingDirective, "using B;"),
Diagnostic(ErrorCode.HDN_UnusedUsingDirective, "using B;").WithLocation(3, 1),
// (4,1): hidden CS8019: Unnecessary using directive.
// using X = C.D;
Diagnostic(ErrorCode.HDN_UnusedUsingDirective, "using X = C.D;"),
Diagnostic(ErrorCode.HDN_UnusedUsingDirective, "using X = C.D;").WithLocation(4, 1),
// (6,1): hidden CS8019: Unnecessary using directive.
// using Z = F<int>;
Diagnostic(ErrorCode.HDN_UnusedUsingDirective, "using Z = F<int>;"),
// (5,1): hidden CS8019: Unnecessary using directive.
// using Y = A::E;
Diagnostic(ErrorCode.HDN_UnusedUsingDirective, "using Y = A::E;"));
Diagnostic(ErrorCode.HDN_UnusedUsingDirective, "using Z = F<int>;").WithLocation(6, 1)
);
}

[Fact]
Expand Down
11 changes: 11 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/LookupTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1424,6 +1424,17 @@ static int Main(string[] args)
Assert.Equal(CandidateReason.OverloadResolutionFailure, symbolInfo.CandidateReason);
}

[Fact]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this test be linked to the issue?

public void TestLookupVerbatimVar()
Copy link
Contributor

Choose a reason for hiding this comment

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

This method is not properly marked as a test method.

{
var source = "class C { public static void Main() { @var v = 1; } }";
CreateCompilationWithMscorlib(source).VerifyDiagnostics(
// (1,39): error CS0246: The type or namespace name 'var' could not be found (are you missing a using directive or an assembly reference?)
// class C { public static void Main() { @var v = 1; } }
Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "@var").WithArguments("var").WithLocation(1, 39)
);
}

private void TestLookupSymbolsNestedNamespaces(List<ISymbol> actual_lookupSymbols)
{
var namespaceX = (NamespaceSymbol)actual_lookupSymbols.Where((sym) => sym.Name.Equals("X") && sym.Kind == SymbolKind.Namespace).Single();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1531,22 +1531,23 @@ static void Main(string[] args)
CreateCompilationWithMscorlib(source).VerifyDiagnostics(
// (9,13): error CS1003: Syntax error, ',' expected
// var x = 1;
Diagnostic(ErrorCode.ERR_SyntaxError, "x").WithArguments(",", ""),
Diagnostic(ErrorCode.ERR_SyntaxError, "x").WithArguments(",", "").WithLocation(9, 13),
// (9,18): error CS1513: } expected
// var x = 1;
Diagnostic(ErrorCode.ERR_RbraceExpected, ";"),
// (6,21): error CS0246: The type or namespace name 'Dictionary<object, object>' could not be found (are you missing a using directive or an assembly reference?)
Diagnostic(ErrorCode.ERR_RbraceExpected, ";").WithLocation(9, 18),
// (6,21): error CS0246: The type or namespace name 'Dictionary<,>' could not be found (are you missing a using directive or an assembly reference?)
// var d = new Dictionary<object, object>()
Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "Dictionary<object, object>").WithArguments("Dictionary<object, object>"),
Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "Dictionary<object, object>").WithArguments("Dictionary<,>").WithLocation(6, 21),
// (8,13): error CS0747: Invalid initializer member declarator
// {"s", 1 },
Diagnostic(ErrorCode.ERR_InvalidInitializerElementInitializer, @"{""s"", 1 }"),
Diagnostic(ErrorCode.ERR_InvalidInitializerElementInitializer, @"{""s"", 1 }").WithLocation(8, 13),
// (9,9): error CS0103: The name 'var' does not exist in the current context
// var x = 1;
Diagnostic(ErrorCode.ERR_NameNotInContext, "var").WithArguments("var"),
Diagnostic(ErrorCode.ERR_NameNotInContext, "var").WithArguments("var").WithLocation(9, 9),
// (9,9): error CS0747: Invalid initializer member declarator
// var x = 1;
Diagnostic(ErrorCode.ERR_InvalidInitializerElementInitializer, "var"));
Diagnostic(ErrorCode.ERR_InvalidInitializerElementInitializer, "var").WithLocation(9, 9)
);
}

[WorkItem(543961, "DevDiv")]
Expand All @@ -1564,25 +1565,26 @@ public static void Main()
CreateCompilationWithMscorlib(source).VerifyDiagnostics(
// (6,25): error CS1513: } expected
// new List<int>() { { { 1 } } };
Diagnostic(ErrorCode.ERR_RbraceExpected, "{"),
Diagnostic(ErrorCode.ERR_RbraceExpected, "{").WithLocation(6, 25),
// (6,25): error CS1003: Syntax error, ',' expected
// new List<int>() { { { 1 } } };
Diagnostic(ErrorCode.ERR_SyntaxError, "{").WithArguments(",", "{"),
Diagnostic(ErrorCode.ERR_SyntaxError, "{").WithArguments(",", "{").WithLocation(6, 25),
// (6,33): error CS1002: ; expected
// new List<int>() { { { 1 } } };
Diagnostic(ErrorCode.ERR_SemicolonExpected, "}"),
Diagnostic(ErrorCode.ERR_SemicolonExpected, "}").WithLocation(6, 33),
// (6,34): error CS1597: Semicolon after method or accessor block is not valid
// new List<int>() { { { 1 } } };
Diagnostic(ErrorCode.ERR_UnexpectedSemicolon, ";"),
Diagnostic(ErrorCode.ERR_UnexpectedSemicolon, ";").WithLocation(6, 34),
// (8,1): error CS1022: Type or namespace definition, or end-of-file expected
// }
Diagnostic(ErrorCode.ERR_EOFExpected, "}"),
// (6,9): error CS0246: The type or namespace name 'List<int>' could not be found (are you missing a using directive or an assembly reference?)
Diagnostic(ErrorCode.ERR_EOFExpected, "}").WithLocation(8, 1),
// (6,9): error CS0246: The type or namespace name 'List<>' could not be found (are you missing a using directive or an assembly reference?)
// new List<int>() { { { 1 } } };
Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "List<int>").WithArguments("List<int>"),
Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "List<int>").WithArguments("List<>").WithLocation(6, 9),
// (6,23): error CS1920: Element initializer cannot be empty
// new List<int>() { { { 1 } } };
Diagnostic(ErrorCode.ERR_EmptyElementInitializer, "{ "));
Diagnostic(ErrorCode.ERR_EmptyElementInitializer, "{ ").WithLocation(6, 23)
);
}

[WorkItem(544484, "DevDiv")]
Expand Down
Loading