Reference Assembly Aliases are not passed to compiler

[rouke-broersma]

Describe the bug
We recently added a package reference alias for a package in Stryker.Core. Mutation testing now fails on Stryker.Core because we don't correctly parse the reference alias.

Logs

Expected behavior
Stryker compiles with Reference Alias since we have support for this.

Additional context
At least currently the References returned from Buildalyzer do not contain references in the format that we try to parse to detect Aliases:

https://github.com/stryker-mutator/stryker-net/blob/master/src/Stryker.Core/Stryker.Core/Initialisation/Buildalyzer/IAnalyzerResultExtensions.cs#L110

Instead PackageReferences does seem to include Aliases:

[Adel-Atzouza]

• PackageReferences can contain aliases and this is implemented
• Assembly References can contain Aliases but this information is not available in Buildalyzer so we don't support these
• ProjectReferences can contain aliases
• multiple aliases can be provided, they must comma separated

[rouke-broersma]

@dupdob do you remember if during implementing support for reference alias if you actually saw references with alias in the format Alias=AssemblyPath.dll? During our testing we could not reproduce this scenario, the only place we could find Aliases was in the PackageReference. We now replaced the current logic with a match on the PackageReference but obviously that means that if a referenced assembly is not a package reference we might break Alias support.

We did try to reproduce aliases on referenced assemblies directly instead of through package reference however the alias was not available in the analyzer result.

[dupdob]

I can't remember sorry. I am not even sure you can directly add reference to assemblies nowadays.

[dupdob]

just for completion: I looked at last year's issue: at that time, I am positive I was able to reproduce the issue (with referenced package) and I did meet this format. Beyond the fact that I remember it, I do not see how I would have been able to guess the appropriate syntax (alias = package) as documentation is lacking on this topic.

My previous comment was referring to 'direct' assembly reference, i.e. pre nuget (for context, I started doing C# with Net V1.1 😉 )

[rouke-broersma]

Yes I thought so too, it's too specific to guess. We could not reproduce it but it made me hesitant to remove it. I also keep this issue open to investigate further.

[dupdob]

thinking about it: it may also be a change in how Buildalyzer reports them!

[dupdob]

I reproduced the issue with integration tests. The aliases are clearly dropped during analysis, either by Buildalyzer or even Roslyn.
The good news is that we can still get them via the compiler command line. We just need to find a safe way to retrieve them.

[rouke-broersma]

Buildalyzer reads the compiler command line arguments passed by msbuild iirc so Buildalyzer probably just doesn't read them correctly, they should be read before Roslyn even does anything.

[dupdob]

It looks like it is using a Roslyn component for C# project, whereas it parses the F# command line directly.
But my first impression may be wrong

[rouke-broersma]

Ah I see what you mean, yes it does seem like Buildalyzer uses a pre-made commandline argument parser supplied by Roslyn to parse the cmd arguments.

[dupdob]

yep, using this class has been introduced in april 24. It looks like parsing was done by Buildalyzer before.

[dupdob]

Analysis

C# native

The C# compiler supports aliases for assembly reference (including multiple aliases for the same assembly). The argument syntax is :/reference:Alias=assemblyPath. As such, it works for project and package references (MsBuild does the resolution part). Note that several aliases can designate the very assembly (this is a valid scenario); the syntax also permits exposing an assembly directly as well as via an alias (not sure this is a valid scenario).

Buildalyzer

Earlier Buildalyzer version had no explicit support for aliases assemblies. As such, these were exposed in the form of alias=assembly, simply because Buildalyzer captured everything after the /reference: parameter.

Buildalyzer V7+ now uses CSharpCommandLineParser to parse the compiler command line. This class does support properly aliases which are exposed via a dedicated property.

Consequences

As Buildalyzer never supported aliases in the first place, this property is not captured and even less reported.
This is a problem for Stryker, as it means Stryker is unable to work on project with aliases anymore.

Solutions

There are two possible approaches to fix this issue, none of them being quick: either by implementing the necessary logic in Stryker or creating a Buildalyzer's PR to fix that.

Implementing it in Stryker

It may be quite simple: the compiler command line is provided as a property in IAnalyzerResult. It then should be just a matter of using CSharpCommandLineParser class to retrieve the expected information. There is one caveat: one must make sure that Buildalyzer exposes the correct command line (csc may be involved several times during the build). If that is not the case, thing will get difficult.

Pros for this approach: Stryker team is autonomous on fixing it; this is probably the fastest way to get a fix. The CommandLineArguments class exposes interesting properties and methods which could be useful for other part of Stryker, such as support for analyzers.

Cons: there is a risk regarding which command line BuilAlyzer exposes; it means Stryker is more involved in the project analysis step than before.

Implementing it in BuildAlyzer

Here we have to sub possibilities

Revert behavior

That is, ensure that aliased reference are returned in the form of alias = assembly. This is a simple fix, one just need to take into account the multi aliases scenario: a single reference may have to be returned several times

Pros: the fix is simple. No change are required on Stryker's end
Cons: reverting the behavior may be identified as a regression for Buildalyzer team and the fix may be rejected. Also, experience shows that there is a several months delay between opening a simple PR and getting it released.

Adding proper support for aliases in Buildalyzer

Here the main effort is to design a non breaking way to expose aliases in IAnalyzerResult. Probably via a new property (something like AssemblyAliases that would return a dictionary of aliases matching their assembly. This means also modifying Stryker to use this property.

Pros: the cleanest way to do this; add a needed feature to Buildalyzer; strengthen the collaboration between both projects.

Cons: will take a long time, as one needs to create the Buildalyzer PR, have it accepted; then wait for it to be released and then re implement alias support in Stryker.

Conclusion

Ultimately, this is a matter of trust in Buildalyzer. If we trust the project team to move this issue swiftly and keep collaborating with Stryker; we should work on the last scenario.
Otherwise, we should work on fixing this in Stryker, which could be a first step towards re implementing the part of Buildalyzer's that are needed for Stryker.

WDYT

[rouke-broersma]

My preference would go to first implement this in Buildalyzer because ultimately we will want it fixed there. If integration takes too long we can decide to implement a temporary solution in Stryker, or we can also accept the current workaround which is that we take the alias from PackageReference and 'assume' that most all users will be using PackageReference anyway. Since in the total lifetime of stryker we have had one total user report needing reference aliases I think we can safely rely on the current behavior for a while.

[dupdob]

Agree. I will see how to expose aliases in BuildAlyzer.
But, the current workaround does not work for ProjectReference (should be a rare occurrence anyway) and it will fail if the package name is different from the assembly name. Likewise, it will probably fails when a package contains more than one assembly.
That being said, there is not much more that can be done about it.

[dupdob]

I opened issues #291 on Buildalyzer and provided a sample solution exhibiting the problem.

Along the way I learned that:

1. every assembly is aliased in an aliased package
2. package dependencies are not aliased

Both make sense, but the second one gave me a hard time finding a way to use an alias for xunit.

[dupdob]

I opened PR 294 that adds alias support for Buildalyzer. Once it is released, we still had to update Stryker to use the newly added IAnalyzerResult property.

[dupdob]

update: PR has been merged! So next Buildalyzer release will containt the fix for AzureFunctions and the support for aliases.