Skip to content

Commit

Permalink
Remove ability to cache interfaces
Browse files Browse the repository at this point in the history
  • Loading branch information
smoogipoo committed Dec 6, 2024
1 parent 83ea26d commit d9528b9
Show file tree
Hide file tree
Showing 16 changed files with 16 additions and 119 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,6 @@ public DependenciesClassCandidate(ClassDeclarationSyntax classSyntax, SemanticMo

protected override void Process(INamedTypeSymbol symbol)
{
// Process any [Cached] attributes on any interface on the class excluding base types.
foreach (var iFace in SyntaxHelpers.GetDeclaredInterfacesOnType(symbol))
{
// Add an entry if this interface has a cached attribute.
foreach (var attrib in iFace.GetAttributes().Where(SyntaxHelpers.IsCachedAttribute))
CachedInterfaces.Add(CachedAttributeData.FromInterfaceOrClass(iFace, attrib));
}

// Process any [Cached] attributes on the class.
foreach (var attrib in symbol.GetAttributes().Where(SyntaxHelpers.IsCachedAttribute))
CachedClasses.Add(CachedAttributeData.FromInterfaceOrClass(symbol, attrib));
Expand Down
45 changes: 0 additions & 45 deletions osu.Framework.Tests/Dependencies/Reflection/CachedAttributeTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -272,17 +272,6 @@ public void TestCacheWithNonAutoGetter()
Assert.Throws<AccessModifierNotAllowedForCachedValueException>(() => DependencyActivator.MergeDependencies(provider, new DependencyContainer()));
}

[Test]
public void TestCachedViaInterface()
{
var provider = new Provider25();

var dependencies = DependencyActivator.MergeDependencies(provider, new DependencyContainer());

Assert.IsNotNull(dependencies.Get<IProviderInterface3>());
Assert.IsNotNull(dependencies.Get<IProviderInterface2>());
}

[Test]
public void TestInheritancePreservesCachingViaBaseType()
{
Expand All @@ -294,18 +283,6 @@ public void TestInheritancePreservesCachingViaBaseType()
Assert.IsNull(dependencies.Get<Provider26>());
}

[Test]
public void TestImplementationOfDerivedInterfacePreservesCaching()
{
var provider = new Provider27();

var dependencies = DependencyActivator.MergeDependencies(provider, new DependencyContainer());

Assert.AreEqual(provider, dependencies.Get<IProviderInterface2>());
Assert.IsNull(dependencies.Get<IProviderInterface4>());
Assert.IsNull(dependencies.Get<Provider27>());
}

private interface IProvidedInterface1
{
}
Expand Down Expand Up @@ -483,30 +460,8 @@ private class Provider24 : IDependencyInjectionCandidate
public object Provided1 => null;
}

private class Provider25 : IProviderInterface3
{
}

private class Provider26 : Provider1
{
}

private class Provider27 : IProviderInterface4
{
}

[Cached]
private interface IProviderInterface3 : IProviderInterface2
{
}

[Cached]
private interface IProviderInterface2 : IDependencyInjectionCandidate
{
}

private interface IProviderInterface4 : IProviderInterface2
{
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -268,17 +268,6 @@ public void TestCacheWithNonAutoGetter()
Assert.Throws<AccessModifierNotAllowedForCachedValueException>(() => DependencyActivator.MergeDependencies(provider, new DependencyContainer()));
}

[Test]
public void TestCachedViaInterface()
{
var provider = new Provider25();

var dependencies = DependencyActivator.MergeDependencies(provider, new DependencyContainer());

Assert.IsNotNull(dependencies.Get<IProviderInterface3>());
Assert.IsNotNull(dependencies.Get<IProviderInterface2>());
}

[Test]
public void TestInheritancePreservesCachingViaBaseType()
{
Expand All @@ -290,18 +279,6 @@ public void TestInheritancePreservesCachingViaBaseType()
Assert.IsNull(dependencies.Get<Provider26>());
}

[Test]
public void TestImplementationOfDerivedInterfacePreservesCaching()
{
var provider = new Provider27();

var dependencies = DependencyActivator.MergeDependencies(provider, new DependencyContainer());

Assert.AreEqual(provider, dependencies.Get<IProviderInterface2>());
Assert.IsNull(dependencies.Get<IProviderInterface4>());
Assert.IsNull(dependencies.Get<Provider27>());
}

private interface IProvidedInterface1
{
}
Expand Down Expand Up @@ -469,30 +446,8 @@ private partial class Provider24 : IDependencyInjectionCandidate
public object Provided1 => null;
}

private partial class Provider25 : IProviderInterface3
{
}

private partial class Provider26 : Provider1
{
}

private partial class Provider27 : IProviderInterface4
{
}

[Cached]
private interface IProviderInterface3 : IProviderInterface2
{
}

[Cached]
private interface IProviderInterface2 : IDependencyInjectionCandidate
{
}

private interface IProviderInterface4 : IProviderInterface2
{
}
}
}
18 changes: 3 additions & 15 deletions osu.Framework/Allocation/CachedAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
namespace osu.Framework.Allocation
{
/// <summary>
/// An attribute that may be attached to a class, interface, field, or property definitions of a <see cref="Drawable"/>
/// An attribute that may be attached to a class, field, or property definitions of a <see cref="Drawable"/>
/// to indicate that the value should be cached as a dependency.
/// Cached values may be resolved through <see cref="BackgroundDependencyLoaderAttribute"/> or <see cref="ResolvedAttribute"/>.
/// </summary>
Expand All @@ -43,15 +43,10 @@ namespace osu.Framework.Allocation
/// See the examples section of the <see cref="Type"/> property documentation for further information.
/// </para>
/// </item>
/// <item>
/// If a class implements an interface annotated with <see cref="CachedAttribute"/>, then instances of that class will cache themselves for their own children using the interface type.
/// As with classes, the <see cref="CachedAttribute"/> is not inherited between interfaces either,
/// but an instance of a class will cache itself to children using all cacheable interface types that it implements.
/// </item>
/// </list>
/// </remarks>
[MeansImplicitUse]
[AttributeUsage(AttributeTargets.Class | AttributeTargets.Field | AttributeTargets.Property | AttributeTargets.Interface, AllowMultiple = true, Inherited = false)]
[AttributeUsage(AttributeTargets.Class | AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = true, Inherited = false)]
public class CachedAttribute : Attribute
{
private static readonly GlobalStatistic<int> count_reflection_attributes = GlobalStatistics.Get<int>("Dependencies", "Reflected [Cached]s");
Expand All @@ -62,7 +57,7 @@ public class CachedAttribute : Attribute
/// The type to cache the value as. If null, the type depends on the type of member that the attribute is placed on:
/// <list type="bullet">
/// <item>In the case of fields and properties, the attribute will use the concrete/most-derived type of the field/property's value.</item>
/// <item>In the case of classes and interfaces, the attribute will use the class/interface type on which the <see cref="CachedAttribute"/> was <em>directly placed</em>.</item>
/// <item>In the case of classes, the attribute will use the class type on which the <see cref="CachedAttribute"/> was <em>directly placed</em>.</item>
/// </list>
/// </summary>
/// <example>
Expand Down Expand Up @@ -91,7 +86,6 @@ public class CachedAttribute : Attribute
/// To achieve that effect, the <see cref="CachedAttribute"/> has to be repeated on class <c>B</c>.
/// </item>
/// </list>
/// <see cref="CachedAttribute"/> placed in interface inheritance hierarchies follows analogous rules to the ones described above for classes.
/// </para>
/// </example>
public Type Type;
Expand Down Expand Up @@ -128,12 +122,6 @@ internal static CacheDependencyDelegate CreateActivator(Type type)

var additionActivators = new List<Action<object, DependencyContainer, CacheInfo>>();

foreach (var iface in type.GetInterfaces())
{
foreach (var attribute in iface.GetCustomAttributes<CachedAttribute>())
additionActivators.Add((target, dc, info) => SourceGeneratorUtils.CacheDependency(dc, type, target, info, attribute.Type ?? iface, attribute.Name, null));
}

foreach (var attribute in type.GetCustomAttributes<CachedAttribute>())
additionActivators.Add((target, dc, info) => SourceGeneratorUtils.CacheDependency(dc, type, target, info, attribute.Type ?? type, attribute.Name, null));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ namespace osu.Framework.Graphics.Containers.Markdown.Footnotes
/// <summary>
/// Visualises a single <see cref="Markdig.Extensions.Footnotes.Footnote"/> within a <see cref="FootnoteGroup"/>.
/// </summary>
[Cached(typeof(IMarkdownTextComponent))]
[Cached(typeof(IMarkdownTextFlowComponent))]
public partial class MarkdownFootnote : CompositeDrawable, IMarkdownTextComponent, IMarkdownTextFlowComponent
{
public readonly Footnote Footnote;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using osu.Framework.Allocation;
using osu.Framework.Graphics.Sprites;

namespace osu.Framework.Graphics.Containers.Markdown
{
[Cached(typeof(IMarkdownTextComponent))]
public interface IMarkdownTextComponent
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using osu.Framework.Allocation;

namespace osu.Framework.Graphics.Containers.Markdown
{
[Cached(Type = typeof(IMarkdownTextFlowComponent))]
public interface IMarkdownTextFlowComponent
public partial interface IMarkdownTextFlowComponent
{
/// <summary>
/// Creates a <see cref="MarkdownTextFlowContainer"/> to display text within this <see cref="IMarkdownTextFlowComponent"/>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ namespace osu.Framework.Graphics.Containers.Markdown
/// code3
///
/// </code>
[Cached(typeof(IMarkdownTextFlowComponent))]
public partial class MarkdownCodeBlock : CompositeDrawable, IMarkdownTextFlowComponent
{
private readonly CodeBlock codeBlock;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ namespace osu.Framework.Graphics.Containers.Markdown
/// <summary>
/// Visualises a markdown text document.
/// </summary>
[Cached(typeof(IMarkdownTextComponent))]
[Cached(typeof(IMarkdownTextFlowComponent))]
public partial class MarkdownContainer : CompositeDrawable, IMarkdownTextComponent, IMarkdownTextFlowComponent
{
private const int root_level = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ namespace osu.Framework.Graphics.Containers.Markdown
/// ## H2
/// ### H3
/// </code>
[Cached(typeof(IMarkdownTextFlowComponent))]
public partial class MarkdownHeading : CompositeDrawable, IMarkdownTextFlowComponent
{
private readonly HeadingBlock headingBlock;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace osu.Framework.Graphics.Containers.Markdown
/// <code>
/// [link text](url)
/// </code>
[Cached(typeof(IMarkdownTextComponent))]
public partial class MarkdownLinkText : CompositeDrawable, IHasTooltip, IMarkdownTextComponent
{
public LocalisableString TooltipText => Url;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ namespace osu.Framework.Graphics.Containers.Markdown
/// <summary>
/// Visualises a paragraph.
/// </summary>
[Cached(typeof(IMarkdownTextFlowComponent))]
public partial class MarkdownParagraph : CompositeDrawable, IMarkdownTextFlowComponent
{
private readonly ParagraphBlock paragraphBlock;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ namespace osu.Framework.Graphics.Containers.Markdown
/// <code>
/// > Quote
/// </code>
[Cached(typeof(IMarkdownTextFlowComponent))]
public partial class MarkdownQuoteBlock : CompositeDrawable, IMarkdownTextFlowComponent
{
private readonly QuoteBlock quoteBlock;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace osu.Framework.Graphics.Containers.Markdown
/// <code>
/// | cell 1 | cell 2 |
/// </code>
[Cached(typeof(IMarkdownTextFlowComponent))]
public partial class MarkdownTableCell : CompositeDrawable, IMarkdownTextFlowComponent
{
public float ContentWidth => textFlow.TotalTextWidth;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ namespace osu.Framework.Graphics.Containers.Markdown
/// <summary>
/// Markdown text flow container.
/// </summary>
[Cached(typeof(IMarkdownTextComponent))]
public partial class MarkdownTextFlowContainer : CustomizableTextContainer, IMarkdownTextComponent
{
public float TotalTextWidth => Padding.TotalHorizontal + FlowingChildren.Sum(x => x.BoundingBox.Size.X);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ namespace osu.Framework.Graphics.Containers.Markdown
/// <summary>
/// Visualises a message that displays when a <see cref="IMarkdownObject"/> doesn't have a visual implementation.
/// </summary>
[Cached(typeof(IMarkdownTextComponent))]
public partial class NotImplementedMarkdown : CompositeDrawable, IMarkdownTextComponent
{
private readonly IMarkdownObject markdownObject;
Expand Down

0 comments on commit d9528b9

Please sign in to comment.