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

Add UsedImplicitly to IDalamudPlugin #1934

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ionite34
Copy link

This adds the UsedImplictly Annotation to IDalamudPlugin to clear up warnings regarding plugin classes not being instantiated.

@ionite34 ionite34 requested a review from a team as a code owner July 15, 2024 19:39
@KazWolfe
Copy link
Member

Does this actually work if your plugin doesn't have JetBrains.Annotations as a dependency? I thought all of these annotations were stripped by the compiler.

@ionite34
Copy link
Author

ionite34 commented Jul 16, 2024

Does this actually work if your plugin doesn't have JetBrains.Annotations as a dependency? I thought all of these annotations were stripped by the compiler.

Actually yeah since the Release builds of Dalamud are used for development this is going to be stripped. (I noticed the existing MeansImplicitUse attribute on [PluginService] doesn't work either). Whether or not the consumer adds the Jetbrains.Annotations dependency the attributes are already stripped and we can't get them back without rebuilding.

The other options are either

  1. Including Jetbrains.Annotations in release builds and compiling with JETBRAINS_ANNOTATIONS condition
  2. Adding a xml external annotations file https://www.jetbrains.com/help/resharper/Code_Analysis__External_Annotations.html and either add it to the release output or make a separate NuGet Package that is referenced by the Dalamud SDK.

I think the xml external annotations file is probably lightweight enough to include in release output for simplicity, and it seems we only need a couple of annotations for implicit use for IDalamudPlugin and the IoC attributes.

For the IDalamudPlugin it would look like this, with similar structure for the IoC injection attributes:

<?xml version="1.0" encoding="utf-8"?>
<assembly name="Dalamud">
    <!-- Mark IDalamudPlugin as implicitly used -->
    <member name="T:Dalamud.Plugin.IDalamudPlugin">
        <attribute ctor="M:JetBrains.Annotations.UsedImplicitlyAttribute.#ctor(JetBrains.Annotations.ImplicitUseKindFlags,JetBrains.Annotations.ImplicitUseTargetFlags)">
            <argument>8</argument>
            <argument>4</argument>
        </attribute>
    </member>
</assembly>

I can change this PR to add the external annotations xml instead if that is acceptable

@KazWolfe
Copy link
Member

Apologies for delay on this. Yeah, I think external annotations XMLs would be totally fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants