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.

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

Diagnostic Improvements for error #0281 #27052

Merged
merged 23 commits into from
Jun 2, 2018
Merged

Diagnostic Improvements for error #0281 #27052

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
37ed85e
Fixed error 0281 and added test coverage.
May 22, 2018
6eb8ea7
Indentation fix
May 23, 2018
de25429
PublicKeyToString no longer an extension method; fixed diagnose in bi…
May 23, 2018
ee189b0
Extracted friendRefNotEqualToThis into local function
May 23, 2018
ab9faaf
Fixed usings
May 23, 2018
af6f5ac
Double blank line fix for Jared
May 23, 2018
0b3c9f1
Resctructured diagnose block to increase readability
May 23, 2018
080de89
Fixed build errors
May 23, 2018
3191a12
Requested changes
May 24, 2018
44e6815
Minor refactoring and fixed return causing incorrect errors
May 24, 2018
e24f1a6
Stack overflow fix
May 24, 2018
cf7eb19
Fix failing tests to conform to new error
May 29, 2018
200acb1
All local tests passing
May 30, 2018
816472a
Extended new error message to PrivateProtected and InternalProtected
May 30, 2018
6f57496
PrivateProtected / InternalProtected Test changes
May 30, 2018
56bb172
Enhanced readability
May 30, 2018
7eba065
Bizarre rabbithole bug
May 30, 2018
72b880a
Indent fixes
May 31, 2018
28f0a80
Re-added attribute check
Jun 1, 2018
15176b2
Restructured to prevent overflow
Jun 1, 2018
6a36501
Test fixes
Jun 1, 2018
e9baab9
Removed Unnecessary changes
Jun 1, 2018
b095767
Oops
Jun 1, 2018
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
29 changes: 20 additions & 9 deletions src/Compilers/CSharp/Portable/Binder/Binder_Lookup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using Microsoft.CodeAnalysis.Collections;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;
Expand Down Expand Up @@ -1148,21 +1147,33 @@ internal SingleLookupResult CheckViability(Symbol symbol, int arity, LookupOptio
return LookupResult.Inaccessible(symbol, diagInfo);
}
else if (!InCref &&
!this.IsAccessible(unwrappedSymbol,
Copy link
Member

Choose a reason for hiding this comment

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

Looks like the old whitespace was correct

!this.IsAccessible(unwrappedSymbol,
RefineAccessThroughType(options, accessThroughType),
out inaccessibleViaQualifier,
ref useSiteDiagnostics,
basesBeingResolved))
{
if (inaccessibleViaQualifier)
{
bool friendRefNotEqualToThis = unwrappedSymbol.ContainingAssembly.GetInternalsVisibleToPublicKeys(this.ContainingType.ContainingAssembly.Name).Any()
&& unwrappedSymbol.DeclaredAccessibility.Equals(Accessibility.Internal);
Copy link
Member

Choose a reason for hiding this comment

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

You should be able to use == instead of .Equals here.


foreach (ImmutableArray<byte> key in unwrappedSymbol.ContainingAssembly.GetInternalsVisibleToPublicKeys(this.ContainingType.ContainingAssembly.Name))
{
friendRefNotEqualToThis = key.SequenceEqual(this.ContainingType.ContainingAssembly.Identity.PublicKey) ? false : friendRefNotEqualToThis;
Copy link
Member

Choose a reason for hiding this comment

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

indentation seems off here...

}
Copy link
Member

Choose a reason for hiding this comment

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

Can be combined with friendRefNotEqualToThis assignment above:

bool friendRefNotEqualToThis = unwrappedSymbol != null && ... &&
        unwrappedSymbol.ContainingAssembly.GetInternalsVisibleToPublicKeys(
            this.ContainingAssembly.Name).Any((key, publicKey) =>
                key.SequenceEqual(publicKey),
                this.ContainingAssembly.Identity.PublicKey);

Copy link
Member

Choose a reason for hiding this comment

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

That would capture this, and capturing lambdas are prohibited in compiler code.

Copy link
Member

@cston cston May 24, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

My mistake, there is no Any overload that takes an additional arg parameter passed to the predicate. @agocke is correct that means the lambda would need to capture a local or this.


if (inaccessibleViaQualifier)
{
diagInfo = diagnose ? new CSDiagnosticInfo(ErrorCode.ERR_BadProtectedAccess, unwrappedSymbol, accessThroughType, this.ContainingType) : null;
}
else
{
}
else if (friendRefNotEqualToThis)
{
diagInfo = diagnose ? new CSDiagnosticInfo(ErrorCode.ERR_FriendRefNotEqualToThis, unwrappedSymbol.ContainingAssembly.Identity.ToString(), this.ContainingType.ContainingAssembly.PublicKey.PublicKeyToString()) : null;
}
else
{
var unwrappedSymbols = ImmutableArray.Create<Symbol>(unwrappedSymbol);
diagInfo = diagnose ? new CSDiagnosticInfo(ErrorCode.ERR_BadAccess, new[] { unwrappedSymbol }, unwrappedSymbols, additionalLocations: ImmutableArray<Location>.Empty) : null;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Consider checking diagnose separately from each individual case. And perhaps combine all checks with ?::

var diagInfo = diagnose ?
    inaccessibleViaQualifier ?
        new CSDiagnosticInfo(...) :
        friendNotEqualToThis ?
            new CSDiagnosticInfo(...) :
            new CSDiagnosticInfo(...) :
        null;


return LookupResult.Inaccessible(symbol, diagInfo);
}
Expand Down Expand Up @@ -1196,7 +1207,7 @@ internal SingleLookupResult CheckViability(Symbol symbol, int arity, LookupOptio
return LookupResult.Good(symbol);
}
}

Copy link
Member

Choose a reason for hiding this comment

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

Accidental whitespace change.

private CSDiagnosticInfo MakeCallMethodsDirectlyDiagnostic(Symbol symbol)
{
Debug.Assert(symbol.MustCallMethodsDirectly());
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/CSharpResources.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/CSharpResources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1123,7 +1123,7 @@
<value>Type does not implement the collection pattern; member has the wrong signature</value>
</data>
<data name="ERR_FriendRefNotEqualToThis" xml:space="preserve">
<value>Friend access was granted by '{0}', but the public key of the output assembly does not match that specified by the attribute in the granting assembly.</value>
<value>Friend access was granted by '{0}', but the public key of the output assembly ('{1}') does not match that specified by the InternalsVisibleTo attribute in the granting assembly.</value>
</data>
<data name="ERR_FriendRefSigningMismatch" xml:space="preserve">
<value>Friend access was granted by '{0}', but the strong name signing state of the output assembly does not match that of the granting assembly.</value>
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Symbols/Source/SourceAssemblySymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1909,7 +1909,7 @@ private void CheckOptimisticIVTAccessGrants(DiagnosticBag bag)

if (conclusion == IVTConclusion.PublicKeyDoesntMatch)
bag.Add(ErrorCode.ERR_FriendRefNotEqualToThis, NoLocation.Singleton,
otherAssembly.Identity);
otherAssembly.Identity.ToString(), this.ContainingAssembly.Identity.ToString());
Copy link
Member

Choose a reason for hiding this comment

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

I don't believe we should be calling otherAssembly.Identity.ToString here. That will cause it to be captured under the current UI culture. Diagnostics can have their messages realized under a different culture.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

toString() has been overridden to be culture-independent.

Copy link
Member

@cston cston May 24, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Why use ToString() here rather than letting the MessageProvider serialize the arguments?

Copy link
Member

Choose a reason for hiding this comment

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

I usually prefer to be explicit on how things get serialized to strings -- Jared's comment reflects that this is usually a very deliberate decision and being explicit signals intent better, IMHO.

else if (conclusion == IVTConclusion.OneSignedOneNot)
bag.Add(ErrorCode.ERR_FriendRefSigningMismatch, NoLocation.Singleton,
otherAssembly.Identity);
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1693,8 +1693,8 @@
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefNotEqualToThis">
<source>Friend access was granted by '{0}', but the public key of the output assembly does not match that specified by the attribute in the granting assembly.</source>
<target state="translated">Sestavení {0} udělilo přístup typu Friend, ale veřejný klíč výstupního sestavení neodpovídá klíči určenému atributem v udělujícím sestavení.</target>
<source>Friend access was granted by '{0}', but the public key of the output assembly ('{1}') does not match that specified by the InternalsVisibleTo attribute in the granting assembly.</source>
<target state="needs-review-translation">Sestavení {0} udělilo přístup typu Friend, ale veřejný klíč výstupního sestavení neodpovídá klíči určenému atributem v udělujícím sestavení.</target>
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefSigningMismatch">
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1693,8 +1693,8 @@
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefNotEqualToThis">
<source>Friend access was granted by '{0}', but the public key of the output assembly does not match that specified by the attribute in the granting assembly.</source>
<target state="translated">Von "{0}" wurde friend-Zugriff gewährt, aber der öffentliche Schlüssel der Ausgabeassembly stimmt nicht mit dem überein, der vom Attribut in der gewährenden Assembly angegeben wird.</target>
<source>Friend access was granted by '{0}', but the public key of the output assembly ('{1}') does not match that specified by the InternalsVisibleTo attribute in the granting assembly.</source>
<target state="needs-review-translation">Von "{0}" wurde friend-Zugriff gewährt, aber der öffentliche Schlüssel der Ausgabeassembly stimmt nicht mit dem überein, der vom Attribut in der gewährenden Assembly angegeben wird.</target>
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefSigningMismatch">
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1693,8 +1693,8 @@
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefNotEqualToThis">
<source>Friend access was granted by '{0}', but the public key of the output assembly does not match that specified by the attribute in the granting assembly.</source>
<target state="translated">{0}' ha concedido acceso de confianza, pero la clave pública del ensamblado de salida no coincide con la especificada por el atributo en el ensamblado de concesión.</target>
<source>Friend access was granted by '{0}', but the public key of the output assembly ('{1}') does not match that specified by the InternalsVisibleTo attribute in the granting assembly.</source>
<target state="needs-review-translation">{0}' ha concedido acceso de confianza, pero la clave pública del ensamblado de salida no coincide con la especificada por el atributo en el ensamblado de concesión.</target>
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefSigningMismatch">
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1693,8 +1693,8 @@
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefNotEqualToThis">
<source>Friend access was granted by '{0}', but the public key of the output assembly does not match that specified by the attribute in the granting assembly.</source>
<target state="translated">Un accès Friend a été concédé par '{0}', mais la clé publique de l'assembly de sortie ne correspond pas à celle spécifiée par l'attribut concédant.</target>
<source>Friend access was granted by '{0}', but the public key of the output assembly ('{1}') does not match that specified by the InternalsVisibleTo attribute in the granting assembly.</source>
<target state="needs-review-translation">Un accès Friend a été concédé par '{0}', mais la clé publique de l'assembly de sortie ne correspond pas à celle spécifiée par l'attribut concédant.</target>
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefSigningMismatch">
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1693,8 +1693,8 @@
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefNotEqualToThis">
<source>Friend access was granted by '{0}', but the public key of the output assembly does not match that specified by the attribute in the granting assembly.</source>
<target state="translated">L'accesso a Friend è stato concesso da '{0}', ma la chiave pubblica dell'assembly di output non corrisponde a quella specificata dall'attributo nell'assembly che ha concesso l'accesso.</target>
<source>Friend access was granted by '{0}', but the public key of the output assembly ('{1}') does not match that specified by the InternalsVisibleTo attribute in the granting assembly.</source>
<target state="needs-review-translation">L'accesso a Friend è stato concesso da '{0}', ma la chiave pubblica dell'assembly di output non corrisponde a quella specificata dall'attributo nell'assembly che ha concesso l'accesso.</target>
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefSigningMismatch">
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1693,8 +1693,8 @@
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefNotEqualToThis">
<source>Friend access was granted by '{0}', but the public key of the output assembly does not match that specified by the attribute in the granting assembly.</source>
<target state="translated">フレンド アクセスのアクセス権は '{0}' によって付与されますが、出力アセンブリの公開キーは、付与するアセンブリで属性によって指定される公開キーと一致しません。</target>
<source>Friend access was granted by '{0}', but the public key of the output assembly ('{1}') does not match that specified by the InternalsVisibleTo attribute in the granting assembly.</source>
<target state="needs-review-translation">フレンド アクセスのアクセス権は '{0}' によって付与されますが、出力アセンブリの公開キーは、付与するアセンブリで属性によって指定される公開キーと一致しません。</target>
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefSigningMismatch">
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1693,8 +1693,8 @@
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefNotEqualToThis">
<source>Friend access was granted by '{0}', but the public key of the output assembly does not match that specified by the attribute in the granting assembly.</source>
<target state="translated">{0}'에서 friend 액세스 권한을 부여했지만, 출력 어셈블리의 공용 키가 부여한 어셈블리의 특성에서 지정된 키와 일치하지 않습니다.</target>
<source>Friend access was granted by '{0}', but the public key of the output assembly ('{1}') does not match that specified by the InternalsVisibleTo attribute in the granting assembly.</source>
<target state="needs-review-translation">{0}'에서 friend 액세스 권한을 부여했지만, 출력 어셈블리의 공용 키가 부여한 어셈블리의 특성에서 지정된 키와 일치하지 않습니다.</target>
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefSigningMismatch">
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1693,8 +1693,8 @@
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefNotEqualToThis">
<source>Friend access was granted by '{0}', but the public key of the output assembly does not match that specified by the attribute in the granting assembly.</source>
<target state="translated">Dostęp do przyjaznego zestawu został udzielony przez „{0}”, ale klucz publiczny zestawu wyjściowego nie jest zgodny z kluczem określonym przez atrybut w zestawie udzielającym dostępu.</target>
<source>Friend access was granted by '{0}', but the public key of the output assembly ('{1}') does not match that specified by the InternalsVisibleTo attribute in the granting assembly.</source>
<target state="needs-review-translation">Dostęp do przyjaznego zestawu został udzielony przez „{0}”, ale klucz publiczny zestawu wyjściowego nie jest zgodny z kluczem określonym przez atrybut w zestawie udzielającym dostępu.</target>
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefSigningMismatch">
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1693,8 +1693,8 @@
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefNotEqualToThis">
<source>Friend access was granted by '{0}', but the public key of the output assembly does not match that specified by the attribute in the granting assembly.</source>
<target state="translated">O acesso Friend foi concedido por "{0}", mas a chave pública do assembly de saída não corresponde àquela especificada pelo atributo no assembly de concessão.</target>
<source>Friend access was granted by '{0}', but the public key of the output assembly ('{1}') does not match that specified by the InternalsVisibleTo attribute in the granting assembly.</source>
<target state="needs-review-translation">O acesso Friend foi concedido por "{0}", mas a chave pública do assembly de saída não corresponde àquela especificada pelo atributo no assembly de concessão.</target>
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefSigningMismatch">
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1693,8 +1693,8 @@
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefNotEqualToThis">
<source>Friend access was granted by '{0}', but the public key of the output assembly does not match that specified by the attribute in the granting assembly.</source>
<target state="translated">Дружественный доступ предоставлен "{0}", однако открытый ключ выходной сборки не соответствует ключу, определенному атрибутом предоставляющей сборки.</target>
<source>Friend access was granted by '{0}', but the public key of the output assembly ('{1}') does not match that specified by the InternalsVisibleTo attribute in the granting assembly.</source>
<target state="needs-review-translation">Дружественный доступ предоставлен "{0}", однако открытый ключ выходной сборки не соответствует ключу, определенному атрибутом предоставляющей сборки.</target>
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefSigningMismatch">
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1693,8 +1693,8 @@
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefNotEqualToThis">
<source>Friend access was granted by '{0}', but the public key of the output assembly does not match that specified by the attribute in the granting assembly.</source>
<target state="translated">{0}' öğesine friend erişimi izni verildi, ancak çıkış derlemesinin ortak anahtarı, izin veren derlemedeki öznitelik tarafından belirtilenle eşleşmiyor.</target>
<source>Friend access was granted by '{0}', but the public key of the output assembly ('{1}') does not match that specified by the InternalsVisibleTo attribute in the granting assembly.</source>
<target state="needs-review-translation">{0}' öğesine friend erişimi izni verildi, ancak çıkış derlemesinin ortak anahtarı, izin veren derlemedeki öznitelik tarafından belirtilenle eşleşmiyor.</target>
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefSigningMismatch">
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1693,8 +1693,8 @@
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefNotEqualToThis">
<source>Friend access was granted by '{0}', but the public key of the output assembly does not match that specified by the attribute in the granting assembly.</source>
<target state="translated">友元访问权限由“{0}”授予，但是输出程序集的公钥与授予程序集中特性指定的公钥不匹配。</target>
<source>Friend access was granted by '{0}', but the public key of the output assembly ('{1}') does not match that specified by the InternalsVisibleTo attribute in the granting assembly.</source>
<target state="needs-review-translation">友元访问权限由“{0}”授予，但是输出程序集的公钥与授予程序集中特性指定的公钥不匹配。</target>
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefSigningMismatch">
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1693,8 +1693,8 @@
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefNotEqualToThis">
<source>Friend access was granted by '{0}', but the public key of the output assembly does not match that specified by the attribute in the granting assembly.</source>
<target state="translated">{0}' 已授與 Friend 存取權限，但輸出組件的公開金鑰不符合授與之組件中屬性所指定的公開金鑰。</target>
<source>Friend access was granted by '{0}', but the public key of the output assembly ('{1}') does not match that specified by the InternalsVisibleTo attribute in the granting assembly.</source>
<target state="needs-review-translation">{0}' 已授與 Friend 存取權限，但輸出組件的公開金鑰不符合授與之組件中屬性所指定的公開金鑰。</target>
<note />
</trans-unit>
<trans-unit id="ERR_FriendRefSigningMismatch">
Expand Down
Loading