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

Jump to bottom

Added PoC tests for adding OnlyVisibleToType to interfaces. #928

Closed
wants to merge 2 commits into from
Closed

Added PoC tests for adding OnlyVisibleToType to interfaces. #928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8cccb49
Added PoC tests for adding OnlyVisibleToType to interfaces.
TimothyJCowen Nov 10, 2023
1d1f654
Adjusted tests for more cases.
TimothyJCowen Nov 13, 2023
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
9 changes: 9 additions & 0 deletions src/D2L.CodeStyle.Analyzers/Diagnostics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -766,5 +766,14 @@ public static class Diagnostics {
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true
);

public static readonly DiagnosticDescriptor TypeNotVisibleToCaller = new DiagnosticDescriptor(
id: "D2L0103",
title: "Type is not visible to caller",
messageFormat: "The type '{0}' has restricted its visibility to an explicit set of callers",
category: "Correctness",
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true
);
}
}
2 changes: 1 addition & 1 deletion src/D2L.CodeStyle.Annotations/Contract/OnlyVisibleToTypeAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
namespace D2L.CodeStyle.Annotations.Contract {

[AttributeUsage(
validOn: AttributeTargets.Constructor | AttributeTargets.Method | AttributeTargets.Property,
validOn: AttributeTargets.Constructor | AttributeTargets.Interface | AttributeTargets.Method | AttributeTargets.Property,
AllowMultiple = true,
Inherited = false
)]
Expand Down
35 changes: 35 additions & 0 deletions src/D2L.CodeStyle.Annotations/Contract/ReleaseVisibilityConstraintsAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
using System;

namespace D2L.CodeStyle.Annotations.Contract {

[AttributeUsage(
validOn: AttributeTargets.Class | AttributeTargets.Interface,
AllowMultiple = true,
Inherited = false
)]
public sealed class ReleaseVisibilityConstraintsAttribute : Attribute {

/// <summary>
/// Removes any restrictions to the visibility of the object
/// which were imposed on the specified base type.
/// </summary>
/// <param name="type">The non-generic type or the generic type definition (e.g. System.Span&lt;&gt;) of the base which holds constraints to visibility.</param>
public ReleaseVisibilityConstraintsAttribute( Type type )
: this( type.FullName, type.Assembly.GetName().Name ) {
}

/// <summary>
/// Removes any restrictions to the visibility of the object
/// which were imposed on the specified base type.
/// </summary>
/// <param name="fullyQualifiedTypeName">The non-generic type or the generic type definition (e.g. System.Span`1) of the base which holds constraints to visibility.</param>
/// <param name="assemblyName">The name of the assembly containing the base type.</param>
public ReleaseVisibilityConstraintsAttribute( string fullyQualifiedTypeName, string assemblyName ) {
FullyQualifiedTypeName = fullyQualifiedTypeName;
AssemblyName = assemblyName;
}

public string FullyQualifiedTypeName { get; }
public string AssemblyName { get; }
}
}
287 changes: 216 additions & 71 deletions tests/D2L.CodeStyle.Analyzers.Test/Specs/OnlyVisibleToAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,24 @@
namespace D2L.CodeStyle.Annotations.Contract {

[AttributeUsage(
validOn: AttributeTargets.Method | AttributeTargets.Property,
validOn: AttributeTargets.Interface | AttributeTargets.Method | AttributeTargets.Property,
AllowMultiple = true,
Inherited = false
)]
public sealed class OnlyVisibleToTypeAttribute : Attribute {
public OnlyVisibleToTypeAttribute( Type type ) { }
public OnlyVisibleToTypeAttribute( string fullyQualifiedTypeName, string assemblyName ) { }
}

[AttributeUsage(
validOn: AttributeTargets.Class | AttributeTargets.Interface,
AllowMultiple = true,
Inherited = false
)]
public sealed class ReleaseVisibilityConstraints : Attribute {
public ReleaseVisibilityConstraints( Type type ) { }
public ReleaseVisibilityConstraints( string fullyQualifiedTypeName, string assemblyName ) { }
}
}

public static class Helpers {
Expand Down Expand Up @@ -276,88 +286,223 @@ public static class TypeOfTargets {
public static void Action() { }
}
}

namespace TestCases {

public static class AllowedTypeOfCaller {
public static void Test() {
TypeOfTargets.Action();
}
}

public static class DisallowedTypeOfCaller {
public static void Test() {
/* MemberNotVisibleToCaller(Action) */ TypeOfTargets.Action() /**/;
}
}

namespace TestCases {

public static class AllowedTypeOfCaller {
public static void Test() {
TypeOfTargets.Action();
}
}

public static class DisallowedTypeOfCaller {
public static void Test() {
/* MemberNotVisibleToCaller(Action) */ TypeOfTargets.Action() /**/;
}
}
}

// ===============================================================================

namespace Targets {

public static class GenericCallerTypes {

[OnlyVisibleToType( typeof( TestCases.AllowedGenericCaller<> ) )]
public static void VisibleByTypeOf() { }

[OnlyVisibleToType( "TestCases.AllowedGenericCaller`1", "OnlyVisibleToAnalyzer" )]
public static void VisibleByQualifiedTypeName() { }

/// <summary>
/// You can only make yourself visible to the generic type definition.
/// </summary>
[OnlyVisibleToType( typeof( TestCases.AllowedGenericCaller<int> ) )]
public static void VisibleButWithGenericTypeArguments() { }
}
}

namespace TestCases {

public static class AllowedGenericCaller<T> {
public static void Test() {
GenericCallerTypes.VisibleByTypeOf();
GenericCallerTypes.VisibleByQualifiedTypeName();
/* MemberNotVisibleToCaller(VisibleButWithGenericTypeArguments) */ GenericCallerTypes.VisibleButWithGenericTypeArguments() /**/;
}
}

public static class DisallowedGenericCaller<T> {
public static void Test() {
/* MemberNotVisibleToCaller(VisibleByTypeOf) */ GenericCallerTypes.VisibleByTypeOf() /**/;
/* MemberNotVisibleToCaller(VisibleByQualifiedTypeName) */ GenericCallerTypes.VisibleByQualifiedTypeName() /**/;
/* MemberNotVisibleToCaller(VisibleButWithGenericTypeArguments) */ GenericCallerTypes.VisibleButWithGenericTypeArguments() /**/;
}
}
}

// ===============================================================================

namespace Targets {

public static class GenericTargetTypes<T> {

[OnlyVisibleToType( typeof( TestCases.AllowedGenericTargetCaller ) )]
public static void VisibleByTypeOf() { }

[OnlyVisibleToType( "TestCases.AllowedGenericTargetCaller", "OnlyVisibleToAnalyzer" )]
public static void VisibleByQualifiedTypeName() { }
}
}

namespace TestCases {

public static class AllowedGenericTargetCaller {
public static void Test() {
GenericTargetTypes<int>.VisibleByTypeOf();
GenericTargetTypes<int>.VisibleByQualifiedTypeName();
}
}

public static class DisallowedGenericTargetCaller {
public static void Test() {
/* MemberNotVisibleToCaller(VisibleByTypeOf) */ GenericTargetTypes<int>.VisibleByTypeOf() /**/;
/* MemberNotVisibleToCaller(VisibleByQualifiedTypeName) */ GenericTargetTypes<int>.VisibleByQualifiedTypeName() /**/;
}
}
}

// ===============================================================================

namespace Targets {

public static class GenericCallerTypes {

[OnlyVisibleToType( typeof( TestCases.AllowedGenericCaller<> ) )]
public static void VisibleByTypeOf() { }

[OnlyVisibleToType( "TestCases.AllowedGenericCaller`1", "OnlyVisibleToAnalyzer" )]
public static void VisibleByQualifiedTypeName() { }

/// <summary>
/// You can only make yourself visible to the generic type definition.
/// </summary>
[OnlyVisibleToType( typeof( TestCases.AllowedGenericCaller<int> ) )]
public static void VisibleButWithGenericTypeArguments() { }
}
}

namespace Targets {
[OnlyVisibleToType( typeof( RestrictedWithInheritance ) )]
[OnlyVisibleToType( typeof( RestrictedWithoutInheritance ) )]
[OnlyVisibleToType( typeof( TestCases.AllowedInheritanceCaller ) )]
public interface IInheritanceInterface {
string SomeProperty;
void SomeMethod( string someValue );
}

public class RestrictedWithInheritance : IInheritanceInterface {
public RestrictedWithInheritance() { }
string SomeProperty => "SomeProperty";
void SomeMethod( string someValue ) { }
}

public class RestrictedWithDoubleInheritance : RestrictedWithInheritance {
public RestrictedWithDoubleInheritance() { }
}

[ReleaseVisibilityConstraints( typeof( IInheritanceInterface ) )]
Copy link
Contributor

@omsmith omsmith Nov 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I feel think this feature becomes a pretty big footgun? If I'm restricting a type, I'm not too worried about what properties/methods I'm adding to it, but then something else might be re-exposing those down the road.

Do we have an example of when we'd use this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up pivoting from having a secondary attribute for precisely this reason - I've created a whole new PoC which leaves it up to the original definition to decide which restrictions are inherited by derived types (all, by default) and which only apply to the class/interface that the restriction was explicitly added to.

As far as an example, yes. This would be used to create a "bridge" between the OrgUnitFileType and UserFileType in the form of an interface (IFileType) which can only be used by explicitly specified consumers. Essentially it is a way for Storageable code to handle files at the very base level without caring about the different entity types (and having to implement code for each type separately). We still want consumers up the chain to be able to use the separate ...FileType classes however, including referencing properties on them that originate from the interface itself, so we don't want the restrictions to be inherited by them.

public class RestrictedWithoutInheritance : IInheritanceInterface {
public RestrictedWithoutInheritance() { }
string SomeProperty => "SomeProperty";
void SomeMethod( string someValue ) { }
}

public class RestrictedWithoutDoubleInheritance : RestrictedWithoutInheritance {
public RestrictedWithoutDoubleInheritance() { }
}
}

namespace TestCases {

public static class AllowedGenericCaller<T> {
public static void Test() {
GenericCallerTypes.VisibleByTypeOf();
GenericCallerTypes.VisibleByQualifiedTypeName();
/* MemberNotVisibleToCaller(VisibleButWithGenericTypeArguments) */ GenericCallerTypes.VisibleButWithGenericTypeArguments() /**/;
public static class AllowedInheritanceCaller {
public static void DirectInstantiation() {
RestrictedWithInheritance withInheritanceObject = new RestrictedWithInheritance();
RestrictedWithDoubleInheritance withDoubleInheritanceObject = new RestrictedWithDoubleInheritance();
RestrictedWithoutInheritance withoutInheritanceObject = new RestrictedWithoutInheritance();
RestrictedWithoutDoubleInheritance withoutDoubleInheritanceObject = new RestrictedWithoutDoubleInheritance();
IInheritanceInterface withInheritanceInterface = new RestrictedWithInheritance();
IInheritanceInterface withDoubleInheritanceInterface = new RestrictedWithDoubleInheritance();
IInheritanceInterface withoutInheritanceInterface = new RestrictedWithoutInheritance();
IInheritanceInterface withoutDoubleInheritanceInterface = new RestrictedWithoutDoubleInheritance();
}
}

public static class DisallowedGenericCaller<T> {
public static void Test() {
/* MemberNotVisibleToCaller(VisibleByTypeOf) */ GenericCallerTypes.VisibleByTypeOf() /**/;
/* MemberNotVisibleToCaller(VisibleByQualifiedTypeName) */ GenericCallerTypes.VisibleByQualifiedTypeName() /**/;
/* MemberNotVisibleToCaller(VisibleButWithGenericTypeArguments) */ GenericCallerTypes.VisibleButWithGenericTypeArguments() /**/;
public static void InterfaceParameter( IInheritanceInterface p ) {
p.SomeMethod( p.SomeProperty );
}
public static void WithInheritanceParameter( RestrictedWithInheritance p ) {
p.SomeMethod( p.SomeProperty );
}
public static void WithDoubleInheritanceParameter( RestrictedWithDoubleInheritance p ) {
p.SomeMethod( p.SomeProperty );
}
public static void WithoutInheritanceParameter( RestrictedWithoutInheritance p ) {
p.SomeMethod( p.SomeProperty );
}
public static void WithoutDoubleInheritanceParameter( RestrictedWithoutDoubleInheritance p ) {
p.SomeMethod( p.SomeProperty );
}
}
}

// ===============================================================================

namespace Targets {

public static class GenericTargetTypes<T> {

[OnlyVisibleToType( typeof( TestCases.AllowedGenericTargetCaller ) )]
public static void VisibleByTypeOf() { }

[OnlyVisibleToType( "TestCases.AllowedGenericTargetCaller", "OnlyVisibleToAnalyzer" )]
public static void VisibleByQualifiedTypeName() { }
public static void GenericInterface<T>() where T : IInheritanceInterface, new() {
T genericObject = new T();
genericObject.SomeMethod( p.SomeProperty );
}
public static void GenericWithInheritance<T>() where T : RestrictedWithInheritance, new() {
T genericObject = new T();
genericObject.SomeMethod( p.SomeProperty );
}
public static void GenericWithDoubleInheritance<T>() where T : RestrictedWithDoubleInheritance, new() {
T genericObject = new T();
genericObject.SomeMethod( p.SomeProperty );
}
public static void GenericWithoutInheritance<T>() where T : RestrictedWithoutInheritance, new() {
T genericObject = new T();
genericObject.SomeMethod( p.SomeProperty );
}
public static void GenericWithoutDoubleInheritance<T>() where T : RestrictedWithoutDoubleInheritance, new() {
T genericObject = new T();
genericObject.SomeMethod( p.SomeProperty );
}
}
}

namespace TestCases {
public static class DisallowedInheritanceCaller {
public static void DirectInstantiation() {
/* TypeNotVisibleToCaller(RestrictedWithInheritance) */ RestrictedWithInheritance /**/ withInheritanceObject = new /* TypeNotVisibleToCaller(RestrictedWithInheritance) */ RestrictedWithInheritance() /**/;
/* TypeNotVisibleToCaller(RestrictedWithDoubleInheritance) */ RestrictedWithDoubleInheritance /**/ withDoubleInheritanceObject = new /* TypeNotVisibleToCaller(RestrictedWithDoubleInheritance) */ RestrictedWithDoubleInheritance() /**/;
RestrictedWithoutInheritance withoutInheritanceObject = new RestrictedWithoutInheritance();
RestrictedWithoutDoubleInheritance withoutDoubleInheritanceObject = new RestrictedWithoutDoubleInheritance();
/* TypeNotVisibleToCaller(IInheritanceInterface) */ IInheritanceInterface /**/ withInheritanceInterface = new /* TypeNotVisibleToCaller(RestrictedWithInheritance) */ RestrictedWithInheritance() /**/;
/* TypeNotVisibleToCaller(IInheritanceInterface) */ IInheritanceInterface /**/ withDoubleInheritanceInterface = new /* TypeNotVisibleToCaller(RestrictedWithDoubleInheritance) */ RestrictedWithDoubleInheritance() /**/;
/* TypeNotVisibleToCaller(IInheritanceInterface) */ IInheritanceInterface /**/ withoutInheritanceInterface = new RestrictedWithoutInheritance();
/* TypeNotVisibleToCaller(IInheritanceInterface) */ IInheritanceInterface /**/ withoutDoubleInheritanceInterface = new RestrictedWithoutDoubleInheritance();
}

public static class AllowedGenericTargetCaller {
public static void Test() {
GenericTargetTypes<int>.VisibleByTypeOf();
GenericTargetTypes<int>.VisibleByQualifiedTypeName();
public static void InterfaceParameter( /* TypeNotVisibleToCaller(IInheritanceInterface) */ IInheritanceInterface /**/ p ) {
p.SomeMethod( p.SomeProperty );
}
public static void WithInheritanceParameter( /* TypeNotVisibleToCaller(RestrictedWithInheritance) */ RestrictedWithInheritance /**/ p ) {
p.SomeMethod( p.SomeProperty );
}
public static void WithDoubleInheritanceParameter( /* TypeNotVisibleToCaller(RestrictedWithDoubleInheritance) */ RestrictedWithDoubleInheritance /**/ p ) {
p.SomeMethod( p.SomeProperty );
}
public static void WithoutInheritanceParameter( RestrictedWithoutInheritance p ) {
p.SomeMethod( p.SomeProperty );
}
public static void WithoutDoubleInheritanceParameter( RestrictedWithoutDoubleInheritance p ) {
p.SomeMethod( p.SomeProperty );
}
}

public static class DisallowedGenericTargetCaller {
public static void Test() {
/* MemberNotVisibleToCaller(VisibleByTypeOf) */ GenericTargetTypes<int>.VisibleByTypeOf() /**/;
/* MemberNotVisibleToCaller(VisibleByQualifiedTypeName) */ GenericTargetTypes<int>.VisibleByQualifiedTypeName() /**/;
public static void GenericInterface<T>() where T : /* TypeNotVisibleToCaller(IInheritanceInterface) */ IInheritanceInterface /**/, new() {
T genericObject = new T();
genericObject.SomeMethod( p.SomeProperty );
}
}
}
public static void GenericWithInheritance<T>() where T : /* TypeNotVisibleToCaller(RestrictedWithInheritance) */ RestrictedWithInheritance/**/, new() {
T genericObject = new T();
genericObject.SomeMethod( p.SomeProperty );
}
public static void GenericWithDoubleInheritance<T>() where T : /* TypeNotVisibleToCaller(RestrictedWithDoubleInheritance) */ RestrictedWithDoubleInheritance /**/, new() {
T genericObject = new T();
genericObject.SomeMethod( p.SomeProperty );
}
public static void GenericWithoutInheritance<T>() where T : RestrictedWithoutInheritance, new() {
T genericObject = new T();
genericObject.SomeMethod( p.SomeProperty );
}
public static void GenericWithoutDoubleInheritance<T>() where T : RestrictedWithoutDoubleInheritance, new() {
T genericObject = new T();
genericObject.SomeMethod( p.SomeProperty );
}
}
}
Loading