Skip to content

Conversation

ViktorHofer
Copy link
Member

@ViktorHofer ViktorHofer commented May 12, 2022

d3af492 made it possible to reference
and package an analyzer via the same msbuild item by setting custom
metadata.

While reviewing other places that could use the AnalyzerReference item,
I realized that using this custom item doesn't provide much value and
creates an artificial difference to the rest of the stack and our customers
as we don't adhere to our own documentation.

Instead, IMHO it makes much more sense to keep using a
ProjectReference item with the documented set of required metadata, to
reference an analyzer and just define an additional custom metadata to
support packaging the analyzer: PackAsAnalyzer.
The reason for that is that the additional metadata explains how the
reference works (no assembly output reference, added as an Analyzer
output item) vs. the AnalyzerReference which is a repo custom item
that doesn't tell you that behind the scenes it actually gets converted
to a ProjectReference with the same metadata as if you would declare
that yourself as a P2P.

To summarize the change:

  1. Consume an analyzer
<!-- Before -->
<AnalyzerReference Include="..." />
<!-- After -->
<ProjectReference Include="..." ReferenceOutputAssembly="false" OutputItemType="Analyzer" />
  1. Pack an analyzer and consume it
<!-- Before -->
<AnalyzerReference Include="..." Pack="true" />
<!-- After -->
<ProjectReference Include="..." ReferenceOutputAssembly="false" OutputItemType="Analyzer" PackAsAnalyzer="true" />
  1. Pack an analyzer without consuming it
<!-- Before -->
<AnalyzerReference Include="..." Pack="true" ReferenceAnalyzer="false" />
<!-- After -->
<ProjectReference Include="..." ReferenceOutputAssembly="false" PackAsAnalyzer="true" />

@ViktorHofer ViktorHofer added this to the 7.0.0 milestone May 12, 2022
@ViktorHofer ViktorHofer requested review from eerhardt and ericstj May 12, 2022 09:24
@ViktorHofer ViktorHofer self-assigned this May 12, 2022
@ghost
Copy link

ghost commented May 12, 2022

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

d3af492 made it possible to reference
and package an analyzer via the same msbuild item by setting custom
metadata.

While reviewing other places that could use the AnalyzerReference item,
I realized that using this custom item doesn't provide much value and
makes us different to rest of the stack / customers as we don't follow
our own documentation.

Instead, IMHO it makes much more sense to keep using a
ProjectReference item with the documented set of required metadata, to
reference an analyzer and just define an additional custom metadata to
support packaging the analyzer: PackAsAnalyzer.
The reason for that is that the additional metadata explains how the
reference works vs. the AnalyzerReference which is a repo custom item
that doesn't tell you that behind the scenes it actually gets converted
to a ProjectReference with the same metadata as if you would declare
that yourself as a P2P.

To summarize the change:

  1. Consume an analyzer
<!-- Before -->
<AnalyzerReference Include="..." />
<!-- After -->
<ProjectReference Include="..." ReferenceOutputAssembly="false" OutputItemType="Analyzer" />
  1. Pack an analyzer and consume it
<!-- Before -->
<AnalyzerReference Include="..." Pack="true" />
<!-- After -->
<ProjectReference Include="..." ReferenceOutputAssembly="false" OutputItemType="Analyzer" PackAsAnalyzer="true" />
  1. Pack an analyzer without consuming it
<!-- Before -->
<AnalyzerReference Include="..." Pack="true" ReferenceAnalyzer="false" />
<!-- After -->
<ProjectReference Include="..." ReferenceOutputAssembly="false" PackAsAnalyzer="true" />
Author: ViktorHofer
Assignees: ViktorHofer
Labels:

area-Infrastructure-libraries

Milestone: 7.0.0

d3af492 made it possible to reference
and package an analyzer via the same msbuild item by setting custom
metadata.

While reviewing other places that could use the AnalyzerReference item,
I realized that using this custom item doesn't provide much value and
makes us different to rest of the stack / customers as we don't follow
our own documentation.

Instead, IMHO it makes much more sense to keep using a
`ProjectReference` item with the documented set of required metadata, to
reference an analyzer and just define an additional custom metadata to
support packaging the analyzer: `PackAsAnalyzer`.
The reason for that is that the additional metadata explains how the
reference works vs. the `AnalyzerReference` which is a repo custom item
that doesn't tell you that behind the scenes it actually gets converted
to a `ProjectReference` with the same metadata as if you would declare
that yourself as a P2P.

To summarize the change:

1. Consume an analyzer
```xml
<!-- Before -->
<AnalyzerReference Include="..." />
<!-- After -->
<ProjectReference Include="..." ReferenceOutputAssembly="false" OutputItemType="Analyzer" />
```

2. Pack an analyzer and consume it
```xml
<!-- Before -->
<AnalyzerReference Include="..." Pack="true" />
<!-- After -->
<ProjectReference Include="..." ReferenceOutputAssembly="false" OutputItemType="Analyzer" PackAsAnalyzer="true" />
```

3. Pack an analyzer without consuming it
```xml
<!-- Before -->
<AnalyzerReference Include="..." Pack="true" ReferenceAnalyzer="false" />
<!-- After -->
<ProjectReference Include="..." ReferenceOutputAssembly="false" PackAsAnalyzer="true" />
```
@ViktorHofer ViktorHofer force-pushed the AnalyzerReferenceRefactoring branch from 020f129 to 2d8d159 Compare May 12, 2022 09:26
@eerhardt
Copy link
Member

eerhardt commented May 12, 2022

I'm not sure I am excited about this approach, since it creates a burden on all the projects that want to consume an analyzer.

<!-- Before -->
<AnalyzerReference Include="..." />
<!-- After -->
<ProjectReference Include="..." ReferenceOutputAssembly="false" OutputItemType="Analyzer" />

The "Before" is much cleaner, and easy to read/understand.

What if we supported 2 scenarios: 1 & 3 above. (I don't think 2 is a common scenario, or really will be used ever. But we will see.):

  1. Consume an analyzer
<!-- Before -->
<AnalyzerReference Include="..." />
<!-- After -->
<AnalyzerReference Include="..." />
  1. Pack an analyzer without consuming it
<!-- Before -->
<AnalyzerReference Include="..." Pack="true" ReferenceAnalyzer="false" />
<!-- After -->
<ProjectReference Include="..." ReferenceOutputAssembly="false" PackAsAnalyzer="true" />

UPDATE: Thinking more about this, how does a "normal dev" reference a local Analyzer they have in their repo in another project? Is it like you are proposing? <ProjectReference Include="..." ReferenceOutputAssembly="false" OutputItemType="Analyzer" /> ? If so, maybe that is the right way to go - that way we don't have special logic, and just do it like a "normal" .NET dev would.

@ViktorHofer ViktorHofer modified the milestones: 7.0.0, 8.0.0 Jul 13, 2022
@ericstj ericstj requested a review from a team July 18, 2022 16:44
@ericstj
Copy link
Member

ericstj commented Jul 18, 2022

@ViktorHofer if this isn't going to be merged in 7.0 then it might be better to close for now.

@ViktorHofer
Copy link
Member Author

Agreed, closing. I will bring this back later.

@ghost ghost locked as resolved and limited conversation to collaborators Aug 17, 2022
@ViktorHofer ViktorHofer deleted the AnalyzerReferenceRefactoring branch November 25, 2022 10:50
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.

3 participants