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

Removed usage of InternalsVisibleTo in all components #446

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
65 changes: 65 additions & 0 deletions components/Animations/src/ArgumentNullExceptionExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;

namespace System;

/// <summary>
/// Throw helper extensions for <see cref="ArgumentNullException"/>.
/// </summary>
internal static class ArgumentNullExceptionExtensions
{
/// <summary>
/// Throws an <see cref="ArgumentNullException"/> for a given parameter name.
/// </summary>
/// <param name="_">Dummy value to invoke the extension upon (always pass <see langword="null"/>.</param>
/// <param name="parameterName">The name of the parameter to report in the exception.</param>
/// <exception cref="ArgumentNullException">Thrown with <paramref name="parameterName"/>.</exception>
[DoesNotReturn]
public static void Throw(this ArgumentNullException? _, string? parameterName)
{
throw new ArgumentNullException(parameterName);
}

/// <summary>
/// Throws an <see cref="ArgumentNullException"/> if <paramref name="argument"/> is <see langword="null"/>.
/// </summary>
/// <param name="_">Dummy value to invoke the extension upon (always pass <see langword="null"/>.</param>
/// <param name="argument">The reference type argument to validate as non-<see langword="null"/>.</param>
/// <param name="parameterName">The name of the parameter with which <paramref name="argument"/> corresponds.</param>
/// <exception cref="ArgumentNullException">Thrown if <paramref name="argument"/> is <see langword="null"/>.</exception>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void ThrowIfNull(this ArgumentNullException? _, [NotNull] object? argument, [CallerArgumentExpression(nameof(argument))] string? parameterName = null)
{
if (argument is null)
{
Throw(parameterName);
}
}

/// <summary>
/// Throws an <see cref="ArgumentNullException"/> if <paramref name="argument"/> is <see langword="null"/>.
/// </summary>
/// <param name="_">Dummy value to invoke the extension upon (always pass <see langword="null"/>.</param>
/// <param name="argument">The pointer argument to validate as non-<see langword="null"/>.</param>
/// <param name="parameterName">The name of the parameter with which <paramref name="argument"/> corresponds.</param>
/// <exception cref="ArgumentNullException">Thrown if <paramref name="argument"/> is <see langword="null"/>.</exception>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static unsafe void ThrowIfNull(this ArgumentNullException? _, [NotNull] void* argument, [CallerArgumentExpression(nameof(argument))] string? parameterName = null)
{
if (argument is null)
{
Throw(parameterName);
}
}

/// <inheritdoc cref="Throw(ArgumentNullException?, string?)"/>
[DoesNotReturn]
private static void Throw(string? parameterName)
{
throw new ArgumentNullException(parameterName);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace CommunityToolkit.WinUI.Animations;
/// <summary>
/// An interface representing a generic model containing info for an abstract keyframe.
/// </summary>
internal interface IKeyFrameInfo
public interface IKeyFrameInfo
{
/// <summary>
/// Gets the easing type to use to reach the new keyframe.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
namespace CommunityToolkit.WinUI.Animations;

/// <inheritdoc cref="NormalizedKeyFrameAnimationBuilder{T}"/>
internal abstract partial class NormalizedKeyFrameAnimationBuilder<T>
public abstract partial class NormalizedKeyFrameAnimationBuilder<T>
{
/// <summary>
/// Gets a <see cref="CompositionAnimation"/> instance representing the animation to start.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
namespace CommunityToolkit.WinUI.Animations;

/// <inheritdoc cref="NormalizedKeyFrameAnimationBuilder{T}"/>
internal abstract partial class NormalizedKeyFrameAnimationBuilder<T>
public abstract partial class NormalizedKeyFrameAnimationBuilder<T>
where T : unmanaged
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace CommunityToolkit.WinUI.Animations;
/// A generic keyframe animation builder.
/// </summary>
/// <typeparam name="T">The type of values being set by the animation being constructed.</typeparam>
internal abstract partial class NormalizedKeyFrameAnimationBuilder<T> : INormalizedKeyFrameAnimationBuilder<T>
public abstract partial class NormalizedKeyFrameAnimationBuilder<T> : INormalizedKeyFrameAnimationBuilder<T>
where T : unmanaged
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@
</PropertyGroup>

<ItemGroup>
<InternalsVisibleTo Include="CommunityToolkit.WinUI.Media" />
<InternalsVisibleTo Include="CommunityToolkit.WinUI.Behaviors" />

<PackageReference Include="PolySharp" Version="1.13.1">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
Expand Down
Copy link
Member

Choose a reason for hiding this comment

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

minor: There's a note on L27 about better support in .NET for this method, so we could do a conditional here so it's better for WinUI?

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, I think we're able to address this now, though we should do it as an improvement from a new Issue/PR.

Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace CommunityToolkit.WinUI.Animations;
/// <summary>
/// An extension <see langword="class"/> for the <see cref="Guid"/> type
/// </summary>
internal static class GuidExtensions
public static class GuidExtensions
{
/// <summary>
/// Returns a <see cref="string"/> representation of a <see cref="Guid"/> only made of uppercase letters
Expand Down
2 changes: 1 addition & 1 deletion components/Animations/src/Xaml/AnimationSet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public sealed class AnimationSet : DependencyObjectCollection
/// <summary>
/// Gets or sets the weak reference to the parent that owns the current animation collection.
/// </summary>
internal WeakReference<UIElement>? ParentReference { get; set; }
public WeakReference<UIElement>? ParentReference { get; set; }
Copy link
Member

Choose a reason for hiding this comment

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

These seems like a case where protected is probably the better intent over public as this is an internal holder of a reference and not something anyone should just get at/use.

Wish we could easily see references from PRs on GitHub and what's using this... 😋

Copy link
Member Author

Choose a reason for hiding this comment

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

Wish we could easily see references from PRs on GitHub and what's using this... 😋

You can do this from the general file browser on github, just select the right branch and file. Works from the "Files Changed" tabs in PRs, too.


/// <inheritdoc cref="AnimationBuilder.Start(UIElement)"/>
/// <exception cref="InvalidOperationException">Thrown when there is no attached <see cref="UIElement"/> instance.</exception>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,5 @@
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>

<!-- TODO: We should figure out a better way to share internal helpers without duplication -->
<!-- e.g. Animations is using ThrowIfNull helper... -->
<InternalsVisibleTo Include="CommunityToolkit.WinUI.Animations" />
<InternalsVisibleTo Include="CommunityToolkit.WinUI.Media" />
</ItemGroup>
</Project>
4 changes: 2 additions & 2 deletions components/Extensions/src/Shadows/AttachedDropShadow.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public sealed class AttachedDropShadow : AttachedShadowBase
#endif

/// <inheritdoc/>
protected internal override bool SupportsOnSizeChangedEvent => true;
public override bool SupportsOnSizeChangedEvent => true;
Copy link
Member

Choose a reason for hiding this comment

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

Curious for all these protected internal cases why protected isn't enough either... @Ryken100 thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

For this SupportsOnSizeChangedEvent, it's referenced in

if (Parent.SupportsOnSizeChangedEvent == true)

Which doesn't derive AttachedDropShadow. It seems like protected internal was allowing protected or internal access, losing internal and being protected means only derived classes can access it. Correct me if I'm wrong, @Ryken100!


private static readonly TypedResourceKey<CompositionRoundedRectangleGeometry> RoundedRectangleGeometryResourceKey = "RoundedGeometry";
private static readonly TypedResourceKey<CompositionSpriteShape> ShapeResourceKey = "Shape";
Expand Down Expand Up @@ -350,7 +350,7 @@ private void CustomMaskedElement_Loaded(object sender, RoutedEventArgs e)
}

/// <inheritdoc/>
protected internal override void OnSizeChanged(AttachedShadowElementContext context, Size newSize, Size previousSize)
public override void OnSizeChanged(AttachedShadowElementContext context, Size newSize, Size previousSize)
{
if (context.SpriteVisual != null)
{
Expand Down
6 changes: 3 additions & 3 deletions components/Extensions/src/Shadows/AttachedShadowBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public Color Color
/// <summary>
/// Gets a value indicating whether or not OnSizeChanged should be called when <see cref="FrameworkElement.SizeChanged"/> is fired.
/// </summary>
protected internal abstract bool SupportsOnSizeChangedEvent { get; }
public abstract bool SupportsOnSizeChangedEvent { get; }

/// <summary>
/// Use this method as the <see cref="PropertyChangedCallback"/> for <see cref="DependencyProperty">DependencyProperties</see> in derived classes.
Expand Down Expand Up @@ -142,7 +142,7 @@ internal void DisconnectElement(FrameworkElement element)
/// Override to handle when the <see cref="AttachedShadowElementContext"/> for an element is being initialized.
/// </summary>
/// <param name="context">The <see cref="AttachedShadowElementContext"/> that is being initialized.</param>
protected internal virtual void OnElementContextInitialized(AttachedShadowElementContext context)
public virtual void OnElementContextInitialized(AttachedShadowElementContext context)
{
OnPropertyChanged(context, OpacityProperty, Opacity, Opacity);
OnPropertyChanged(context, BlurRadiusProperty, BlurRadius, BlurRadius);
Expand Down Expand Up @@ -288,7 +288,7 @@ protected virtual void OnPropertyChanged(AttachedShadowElementContext context, D
/// <param name="context">The <see cref="AttachedShadowElementContext"/> for the <see cref="FrameworkElement"/> firing its SizeChanged event</param>
/// <param name="newSize">The new size of the <see cref="FrameworkElement"/></param>
/// <param name="previousSize">The previous size of the <see cref="FrameworkElement"/></param>
protected internal virtual void OnSizeChanged(AttachedShadowElementContext context, Size newSize, Size previousSize)
public virtual void OnSizeChanged(AttachedShadowElementContext context, Size newSize, Size previousSize)
{
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -278,36 +278,36 @@ public bool TryGetResource<T>(string key, out T? resource)
/// </summary>
/// <typeparam name="T">The type of the resource being added.</typeparam>
/// <returns>The resource that was added</returns>
internal T AddResource<T>(TypedResourceKey<T> key, T resource)
public T AddResource<T>(TypedResourceKey<T> key, T resource)
where T : notnull => AddResource(key.Key, resource);

/// <summary>
/// Retrieves a resource with the specified key and type if it exists
/// </summary>
/// <typeparam name="T">The type of the resource being retrieved.</typeparam>
/// <returns>True if the resource exists, false otherwise</returns>
internal bool TryGetResource<T>(TypedResourceKey<T> key, out T? resource) => TryGetResource(key.Key, out resource);
public bool TryGetResource<T>(TypedResourceKey<T> key, out T? resource) => TryGetResource(key.Key, out resource);

/// <summary>
/// Retries a resource with the specified key and type
/// </summary>
/// <typeparam name="T">The type of the resource being retrieved.</typeparam>
/// <returns>The resource if it exists or a default value.</returns>
internal T? GetResource<T>(TypedResourceKey<T> key) => GetResource<T>(key.Key);
public T? GetResource<T>(TypedResourceKey<T> key) => GetResource<T>(key.Key);

/// <summary>
/// Removes an existing resource with the specified key and type
/// </summary>
/// <typeparam name="T">The type of the resource being removed.</typeparam>
/// <returns>The resource that was removed, if any</returns>
internal T? RemoveResource<T>(TypedResourceKey<T> key) => RemoveResource<T>(key.Key);
public T? RemoveResource<T>(TypedResourceKey<T> key) => RemoveResource<T>(key.Key);

/// <summary>
/// Removes an existing resource with the specified key and type, and <see cref="IDisposable.Dispose">disposes</see> it
/// </summary>
/// <typeparam name="T">The type of the resource being removed.</typeparam>
/// <returns>The resource that was removed, if any</returns>
internal T? RemoveAndDisposeResource<T>(TypedResourceKey<T> key)
public T? RemoveAndDisposeResource<T>(TypedResourceKey<T> key)
where T : IDisposable => RemoveAndDisposeResource<T>(key.Key);

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion components/Extensions/src/Shadows/TypedResourceKey.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace CommunityToolkit.WinUI;
/// A generic class that can be used to retrieve keyed resources of the specified type.
/// </summary>
/// <typeparam name="TValue">The <see cref="Type"/> of resource the <see cref="TypedResourceKey{TValue}"/> will retrieve.</typeparam>
internal sealed class TypedResourceKey<TValue>
public sealed class TypedResourceKey<TValue>
{
/// <summary>
/// Initializes a new instance of the <see cref="TypedResourceKey{TValue}"/> class with the specified key.
Expand Down
65 changes: 65 additions & 0 deletions components/Media/src/ArgumentNullExceptionExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;

namespace System;

/// <summary>
/// Throw helper extensions for <see cref="ArgumentNullException"/>.
/// </summary>
internal static class ArgumentNullExceptionExtensions
{
/// <summary>
/// Throws an <see cref="ArgumentNullException"/> for a given parameter name.
/// </summary>
/// <param name="_">Dummy value to invoke the extension upon (always pass <see langword="null"/>.</param>
/// <param name="parameterName">The name of the parameter to report in the exception.</param>
/// <exception cref="ArgumentNullException">Thrown with <paramref name="parameterName"/>.</exception>
[DoesNotReturn]
public static void Throw(this ArgumentNullException? _, string? parameterName)
{
throw new ArgumentNullException(parameterName);
}

/// <summary>
/// Throws an <see cref="ArgumentNullException"/> if <paramref name="argument"/> is <see langword="null"/>.
/// </summary>
/// <param name="_">Dummy value to invoke the extension upon (always pass <see langword="null"/>.</param>
/// <param name="argument">The reference type argument to validate as non-<see langword="null"/>.</param>
/// <param name="parameterName">The name of the parameter with which <paramref name="argument"/> corresponds.</param>
/// <exception cref="ArgumentNullException">Thrown if <paramref name="argument"/> is <see langword="null"/>.</exception>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void ThrowIfNull(this ArgumentNullException? _, [NotNull] object? argument, [CallerArgumentExpression(nameof(argument))] string? parameterName = null)
{
if (argument is null)
{
Throw(parameterName);
}
}

/// <summary>
/// Throws an <see cref="ArgumentNullException"/> if <paramref name="argument"/> is <see langword="null"/>.
/// </summary>
/// <param name="_">Dummy value to invoke the extension upon (always pass <see langword="null"/>.</param>
/// <param name="argument">The pointer argument to validate as non-<see langword="null"/>.</param>
/// <param name="parameterName">The name of the parameter with which <paramref name="argument"/> corresponds.</param>
/// <exception cref="ArgumentNullException">Thrown if <paramref name="argument"/> is <see langword="null"/>.</exception>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static unsafe void ThrowIfNull(this ArgumentNullException? _, [NotNull] void* argument, [CallerArgumentExpression(nameof(argument))] string? parameterName = null)
{
if (argument is null)
{
Throw(parameterName);
}
}

/// <inheritdoc cref="Throw(ArgumentNullException?, string?)"/>
[DoesNotReturn]
private static void Throw(string? parameterName)
{
throw new ArgumentNullException(parameterName);
}
}
1 change: 1 addition & 0 deletions components/Media/src/CommunityToolkit.WinUI.Media.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

<!-- Rns suffix is required for namespaces shared across projects. See https://github.com/CommunityToolkit/Labs-Windows/issues/152 -->
<RootNamespace>CommunityToolkit.WinUI.MediaRns</RootNamespace>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<PackageReadmeFile>ReadMe.md</PackageReadmeFile>
</PropertyGroup>

Expand Down
6 changes: 3 additions & 3 deletions components/Media/src/Shadows/AttachedCardShadow.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,10 @@ public InnerContentClipMode InnerContentClipMode
#endif

/// <inheritdoc/>
protected internal override bool SupportsOnSizeChangedEvent => true;
public override bool SupportsOnSizeChangedEvent => true;

/// <inheritdoc/>
protected internal override void OnElementContextInitialized(AttachedShadowElementContext context)
public override void OnElementContextInitialized(AttachedShadowElementContext context)
{
UpdateVisualOpacityMask(context);
base.OnElementContextInitialized(context);
Expand Down Expand Up @@ -330,7 +330,7 @@ protected override void SetElementChildVisual(AttachedShadowElementContext conte
}

/// <inheritdoc />
protected internal override void OnSizeChanged(AttachedShadowElementContext context, Size newSize, Size previousSize)
public override void OnSizeChanged(AttachedShadowElementContext context, Size newSize, Size previousSize)
{
Vector2 sizeAsVec2 = newSize.ToVector2();

Expand Down
2 changes: 1 addition & 1 deletion tooling
Loading