Skip to content

Conversation

@arontsang
Copy link
Contributor

What kind of change does this PR introduce?

Feature. This adds a new Roslyn code analyzer that ensures that the [Reactive] attribute is added only to classes that implement IReactiveObject.

Unfortunately, I do not have the expertise to wrap this up in a nuget package

What is the current behavior?

When a user tries to add the [Reactive] attribute to a property on a class that does not implement IReactiveObject, the weaving silently fails.

What is the new behavior?

When a user tries to add the [Reactive] attribute to a property on a class that does not implement IReactiveObject, the IDE should give a compile time error.

What might this PR break?
None

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

@arontsang arontsang requested review from a team October 9, 2019 16:29
@todo
Copy link

todo bot commented Oct 9, 2019

Consider registering other actions that act on syntax instead of or in addition to symbols

// TODO: Consider registering other actions that act on syntax instead of or in addition to symbols
// See https://github.com/dotnet/roslyn/blob/master/docs/analyzers/Analyzer%20Actions%20Semantics.md for more information
context.RegisterSyntaxNodeAction(AnalyzeAttribute, SyntaxKind.Attribute);
}
private static void AnalyzeAttribute(SyntaxNodeAnalysisContext context)


This comment was generated by todo based on a TODO comment in 61b7e15 in #2199. cc @arontsang.

@dnfclas
Copy link

dnfclas commented Oct 9, 2019

CLA assistant check
All CLA requirements met.

@RLittlesII RLittlesII changed the title Add Roslyn Analyzer for Fody plugin feature: Add Roslyn Analyzer for Fody plugin Oct 9, 2019
Copy link
Member

@RLittlesII RLittlesII left a comment

Choose a reason for hiding this comment

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

Currently the build script won't build or test this project. It should be added to the white list.

ReactiveUI/build.cake

Lines 16 to 30 in 1357b14

var packageWhitelist = new List<FilePath>
{
MakeAbsolute(File("./src/ReactiveUI/ReactiveUI.csproj")),
MakeAbsolute(File("./src/ReactiveUI.Testing/ReactiveUI.Testing.csproj")),
MakeAbsolute(File("./src/ReactiveUI.Events/ReactiveUI.Events.csproj")),
MakeAbsolute(File("./src/ReactiveUI.Events.XamEssentials/ReactiveUI.Events.XamEssentials.csproj")),
MakeAbsolute(File("./src/ReactiveUI.Events.XamForms/ReactiveUI.Events.XamForms.csproj")),
MakeAbsolute(File("./src/ReactiveUI.Fody/ReactiveUI.Fody.csproj")),
MakeAbsolute(File("./src/ReactiveUI.Fody.Helpers/ReactiveUI.Fody.Helpers.csproj")),
MakeAbsolute(File("./src/ReactiveUI.AndroidSupport/ReactiveUI.AndroidSupport.csproj")),
MakeAbsolute(File("./src/ReactiveUI.AndroidX/ReactiveUI.AndroidX.csproj")),
MakeAbsolute(File("./src/ReactiveUI.XamForms/ReactiveUI.XamForms.csproj")),
MakeAbsolute(File("./src/ReactiveUI.Uno/ReactiveUI.Uno.csproj")),
MakeAbsolute(File("./src/ReactiveUI.Blazor/ReactiveUI.Blazor.csproj")),
};

@arontsang arontsang requested review from a team and glennawatson and removed request for a team October 10, 2019 15:59
@glennawatson glennawatson merged commit e24fbdd into reactiveui:master Oct 12, 2019
@lock lock bot locked and limited conversation to collaborators Jan 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants