[Bug] MCT001 doesn't recognize initialization in a library

[bakerhillpins]

Description

Update to the latest CommunityToolkit.Maui Version="1.1.0" and my project now fails build due to error:

MCT001: .UseMauiCommunityToolkit() must be chained to .UseMauiApp<T>()

While I like the fact that the library is looking over my shoulder to ensure I'm calling this, it really should be written to understand the dependency chain when looking for the call to the initialization routine. My software architecture is loosely coupled allowing libraries of functionality to be added individually to the main/exe project. Anytime a individual library is added, a corresponding fluent init method is called (just as you're doing with the toolkit). So in my situation, one of my UI libraries takes advantage of the Toolkit, and thus its MauiApp Fluent Initialization method calls .UseMauiCommunityToolkit() directly. My main/exe project therefore calls the .UseMauiCommunityToolkit() method indirectly. However, because your initialization test is transient, it is failing the top level project when in fact it should be stopping at the library level.

This also brings up the question - can the .UseMauiCommunityToolkit() method be called multiple times (multiple decoupled libraries in a project use the toolkit) without undesired side effects such as service registrations, etc.

Steps to Reproduce

1. Create a MAUI app that references a MAUI Library.
2. Inside that library, add a reference to CommunityToolkit.Maui Version="1.1.0"
3. Call .UseMauiCommunityToolkit() from inside the Library
4. Build and see error

Expected Behavior

No build failure

Actual Behavior

build failure

Basic Information

• Version with issue: 1.1.0
• Last known good version: 1.0.0

Workaround

Add MCT001 to Suppress specific warnings in project Build>Errors and Warnings section

Demo project

I created this project with Microsoft Visual Studio Professional 2022 (64-bit) - Preview
Version 17.3.0 Preview 4.0

MauiAppInitBug.zip

[pictos]

@bakerhillpins
Could you attach a small repro?

[bakerhillpins]

Ok, I've added a simple demo project. It's actually highlighted another problem with the Code generator for this error:

All you have to have is the text .UseMauiCommunityToolkit() somewhere in the method chain for the builder and your test will evaluate to no error. Thus the following code does not produce the MCT001 error even though the method is commented out.

        var builder = MauiApp.CreateBuilder();
	builder
	     .UseMauiApp<App>()
            .ConfigureLibrary()
            //.UseMauiCommunityToolkit()
            .ConfigureFonts(fonts =>
                            {
                                fonts.AddFont("OpenSans-Regular.ttf", "OpenSansRegular");
                                fonts.AddFont("OpenSans-Semibold.ttf", "OpenSansSemibold");
                            });

but this snippet does:

        //.UseMauiCommunityToolkit()
        var builder = MauiApp.CreateBuilder();
	builder
	     .UseMauiApp<App>()
            .ConfigureLibrary()
            .ConfigureFonts(fonts =>
                            {
                                fonts.AddFont("OpenSans-Regular.ttf", "OpenSansRegular");
                                fonts.AddFont("OpenSans-Semibold.ttf", "OpenSansSemibold");
                            });

[bakerhillpins]

@pictos This issue is not resolved. It would appear that you didn't read the issue description as the problem reported has to do with transient references to the library and is not fixed.

The issue with testing for actual code is something I found when I was creating a demo project.

[pictos]

@bakerhillpins I'm sorry, but I'm guided by samples. You provide a snippet and I confirmed that the issue happens in that case, so I fixed that. Didn't pay attention to the lib part.

[bakerhillpins]

Please don't let this die on the vine, The fact that I am forced to register the toolkit at the top level of my application breaks my decoupled application framework code by forcing me to pull a dependency up through my project structure.

[bijington]

I've been thinking on this issue. My analyser knowledge is limited but I suspected that working out that ConfigureLibrary calls UseMauiCommunityToolkit might be complicated. Especially if we consider there might be another layer in the way. Is this correct?

If so could we introduce an attribute the could be added to the ConfigureLibrary method so we know it is initialising our toolkit? Something like [InitializeMauiCommunityToolkit]. Is this feasible or silly?

[bakerhillpins]

@bijington My source generator knowledge is limited, though I have authored some for our development chain, and I do believe that this will be a difficult problem to solve. I actually tried (without success) to locate a discussion about this subject that I recall landing on where this was abandoned.

However, I'd have thought that this source generator should only be called by the project that actually references the toolkit as that's where you'd expect the Initialization call to occur, but you can't since it can be done anywhere according to developer preference. Assuming the referencing project case, it becomes an exhaustive search algo to find any code that references the MaulAppBuilder object. However, you can imagine how many different ways a user might pass or otherwise obscure the basic fluent initialization chain. This continues on and on trying to chase the myriad of ways a developer COULD make the Toolkit initialization call. I think it's a great idea in concept, but this basic approach seems like it's going to get lost in corner cases that make it's implementation difficult and the "bang for the buck" really low.

[pictos]

@bijington yes, we can add an attribute and look for that. That should work similarly to how the analyzer detects Obsolete attributes in the code base.

@bakerhillpins this isn't a Source Generator issue and a Roslyn Analyzer issue. We don't want to cover all scenarios, if the user is doing something fancy he/she can disable our analyzer easily, as you can see in the screenshot below:

[bakerhillpins]

@pictos My understanding is that Source Generators are based in the Roslyn Analyzer arch. So same basic premise behind both.

[pictos]

Ooh i see now

[bakerhillpins]

if the user is doing something fancy he/she can disable our analyzer easily, as you can see in the screenshot below:

@pictos If I understand you correctly you're suggesting that inclusion of the library in a decoupled design (Among other designs) would require an extra step to disable the build error - Effectively an Opt Out feature? From my perspective that's certainly an undesirable behavior. I'd hate to have to have users include a library and then be met with an error that's not technically necessary or even correct and then must be disabled in each project that includes the library. Which isn't actually the obvious solution to the problem.

This discussion is re-enforcing my opinion that this "feature" has little value. It's more of an annoyance than the problem it's designed to solve. E.g.

• A developer might get caught by the situation it's designed to avoid a single time early on in development. (This is minimal impact)
• It becomes a re-occurring build failure when in fact the problem probably doesn't actually exist because the init call IS made. (This is a False error condition) This SHOULD NOT ever occur IMHO
• needs to be disabled in configuration to fix a build error? This is completely counterintuitive for any developer. I never start debugging a build error by saying "How do I disable this error?". (This goes against standard practice)

[pictos]

@bakerhillpins if in your lib, you don't call the UseMauiCommunityToolkit and the application developer call that method on the MauiProgram class, will the lib work? Or is it required to call our initializer inside the lib's init method?

For a bit more context, at least for me, never thought that lib. authors will reference our toolkit, so I never account this scenario in the analyzer.

I disagree this adds little value, when this came public on Twitter other lib authors thought this a good idea and want to implement it on their projects. Isn't because you face an issue this adds little value (; , let's think a workaround for now, that way you're not blocked and later think in how we can solve the issue for libs that reference our toolkit.

[bakerhillpins]

@bakerhillpins if in your lib, you don't call the UseMauiCommunityToolkit and the application developer call that method on the MauiProgram class, will the lib work? Or is it required to call our initializer inside the lib's init method?

Yes it will work that way but that brakes the encapsulation/isolation of the application component library (which references the toolkit) by forcing an unnecessary dependency between the application and the toolkit (which is only necessary for the component Library). The application that references the component Library shouldn't have to manage implementation details of the reference. This is the same reason a developer would specify an Interface/API publicly and hide the details of the implementation behind it. No consumer of any API should have to take implementation dependencies directly in order to use the Interface/API. This is a basic software design principle. Proper test design would also make this situation evident.

Regardless, the MCT001 code is INCORRECTLY reporting an error. This is a false positive and should never be fixed by disabling the error.

For a bit more context, at least for me, never thought that lib. authors will reference our toolkit, so I never account this scenario in the analyzer.

Might I suggest reading up on software architecture design then, starting with this document: The Onion Architecture These are fundamental principles of design, allowing for reuse (This is a Library after all and should be designed for reuse). This is a great starting piece and you can search and build from there. Any other documentation on Interface/API (Not User Interface Design although it is related) or design for testing is also going to discuss the subject.

I disagree this adds little value, when this came public on Twitter other lib authors thought this a good idea and want to implement it on their projects. Isn't because you face an issue this adds little value (; , let's think a workaround for now, that way you're not blocked and later think in how we can solve the issue for libs that reference our toolkit.

It's nice that feedback was solicited for the idea. However, Twitter is a great place for opinions but rarely anything else. Was there a discussion raised here, in the project, where users could discuss the merits? I was unable to locate one. Microsoft themselves follow this model for good reason. The value of this feature is not subjective, it's easy to determine. It's surface area is directly correlated to the number of problems it solves for the user of the Library.

• In this case it solves exactly ONE problem. The developer forgot to call the Initialization method.
• When could this error occur. This is also simple. It happens EARLY in the development/use cycle because it's necessary for the Library to function. It is unlikely that the developer will spend months coding a solution that uses the library before trying any individual feature out thus the code to evaluate is small.
• Is it likely to reoccur. No, this is a startup time issue and that code is unlikely to be modified as the library user adds more features which leverage the library.

Thus the surface area where this issue could effect an end user of the library is LIMITED.

The complexity of this problem is HIGH since the number of permutations for a valid call to the initialization method is HIGH and requires a significant amount of complex code to cover the corner cases. This fact that you've already suggested overriding the error as the fix suggests that the solution is overly burdensome on the library developers and you're trying to ignore it.

Low surface area and High complexity suggests that this has limited impact for users and high impact on library maintenance. IMHO this feature should be removed.

[bijington]

I don't believe we did discuss this feature anywhere here because the main aim was to reduce the number of issues that were being raised because developers were not reading the documentation. Therefore the goal of the feature was to reduce burden on the maintainers with the added benefit of the tooling informing the developers of the steps they had missed.

I can't speak for all of the maintainers but I believe there is still value in keeping this feature especially as we have not had an issue raised where a user has not initialised the toolkit since the introduction of this feature.

All that being said it sounds like we missed a use case and that itself should be solvable.

To confirm the current state of things, are you seeing this issue inside a single solution (you have the library and app project in the same solution) or are you building a library for others to consume? The former has a workaround in that you can suppress the error until a possible solution is implemented, the latter still does but I appreciate it moves the suppression onto your consumers which is less ideal.

[bakerhillpins]

I can't speak for all of the maintainers but I believe there is still value in keeping this feature especially as we have not had an issue raised where a user has not initialised the toolkit since the introduction of this feature.

In the end, as the maintainers you have the ultimate decision here and I respect that. I'll make my use recommendations/decisions based upon needs for our business. I can sympathize with the situation that arises from the predominate behavior of software developers these days.

To confirm the current state of things, are you seeing this issue inside a single solution (you have the library and app project in the same solution) or are you building a library for others to consume? The former has a workaround in that you can suppress the error until a possible solution is implemented, the latter still does but I appreciate it moves the suppression onto your consumers which is less ideal.

The library is a separate NuGet package designed to be consumed on an as needed basis by multiple business applications. The example project included the library in the same solution because it was easy to demonstrate the problem. The issue remains the same regardless of consumption as a project or NuGet package.

It's unclear to me if the consumption of the Toolkit as a transient reference was part of the feature design. I think its clear that this will have an effect on ANY other software component provider who consumes your library as a component of their library/NuGet package.

[TheCodeTraveler]

Hey Gang! This is a challenging problem to solve. Implementing Roslyn Analyzers and Code Fixes is a new challenge for all C# developers, and I promise you that if I knew how to solve this Issue I would've implemented it already, just like I'm sure you, @bakerhillpins, would've submitted a PR if you knew the solution as well.

We will be keeping this Analyzer because UseMauiCommunityToolkit() is required for our library to work properly.

@bakerhillpins we are aware of this Issue. It is on our radar. If you are able to update the Analyzer to solve it, please submit a PR.

Please keep this conversation scoped to solutions to the Issue.

[vhugogarcia]

@brminnick I think we can close this issue, and move it to a discussion (if still needed). Pedro already solved an issue when comments are set in the initialization, which I know it was not part of the original issue reported. However, I think this is a requested feature to support MCT in a library and not an actual issue, and can be discussed first, so all can come up with ideas about which can be the best implementation for including MCT in a library (if actually, it is needed for decoupled environments).