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 analyzer template #9789

Conversation

YuliiaKovalova
Copy link
Member

@YuliiaKovalova YuliiaKovalova commented Feb 27, 2024

Related

#9633

Details

Add a basic content for a custom analyzer.

Given the multitude of prerequisites necessary for this operation, this template aims to simplify the process by automating the following tasks:

  1. Copying third-party dependencies.
  2. Invocation of intrinsic function [MSBuild]::RegisterAnalyzer(string) from a custom .props file.
  3. Modification of the output structure for NuGet packages, shifting from the standard /lib/platform/*.dll to lib/*.dll.

Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

Great!!

Do you plan to add a reference of Microsoft.Build package and add a dummy implementation? That would be likely helpfull as well.

The version of the PackageDependency can be a parameter in template (defaulting to latest current release, https://github.com/dotnet/msbuild/blob/main/eng/Versions.props#L5); the sample implementation can be part of the Microsoft.Build, added here as link - ensuring the code is up to date. Though I haven't thought about that too deeply - if it turns complicated, feel free to merge the current version and create separate workitem for improvements

@YuliiaKovalova
Copy link
Member Author

Great!!

Do you plan to add a reference of Microsoft.Build package and add a dummy implementation? That would be likely helpfull as well.

The version of the PackageDependency can be a parameter in template (defaulting to latest current release, https://github.com/dotnet/msbuild/blob/main/eng/Versions.props#L5); the sample implementation can be part of the Microsoft.Build, added here as link - ensuring the code is up to date. Though I haven't thought about that too deeply - if it turns complicated, feel free to merge the current version and create separate workitem for improvements

Jan, I have added package reference to Microsoft.Build

Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

Do we need an explicit pack execution for this - so that actual package is produced as part of the build?

"type": "parameter",
"description": "Overrides the default Microsoft.Build version where analyzer's interfaces are placed",
"datatype": "text",
"defaultValue": "17.9.5",
Copy link
Member

Choose a reason for hiding this comment

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

Something for the future iteration - the version here should be in sync with Version.props - we can either ensure that during pcaking, or just simply ad a manual steps to our release checklist

Copy link
Member Author

Choose a reason for hiding this comment

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

@GangWang01, could you take care of it? the first option is preferable. Feel free to create a separate work item for that.

Copy link
Member

Choose a reason for hiding this comment

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

@GangWang01 - would you be able to handle #9915 as well? Feel free to contact me with any questions

Copy link
Member

Choose a reason for hiding this comment

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

Created #9923. And self assigned with both issues.

Copy link
Member

Choose a reason for hiding this comment

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

Something for the future iteration - the version here should be in sync with Version.props - we can either ensure that during pcaking, or just simply ad a manual steps to our release checklist

@JanKrivanek Could you give me more info how to add a manual steps to our release checklist?

Copy link
Member Author

Choose a reason for hiding this comment

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

please include this info to this document
#9958

template_feed/Microsoft.AnalyzerTemplate/Analyzer1.cs Outdated Show resolved Hide resolved
Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

Great work - thanks!!

I added #9915 to handle the packing

@YuliiaKovalova YuliiaKovalova merged commit 31a2a0e into dotnet:exp/build-analyzers Mar 25, 2024
9 checks passed
JanKrivanek added a commit that referenced this pull request Apr 15, 2024
* Initial demonstration version

* Make analyzer test use bootstrap properly (#9733)

* Hook analyzers stats stub

* Fix unit tests by explicitly opting into analysis

* Disable build acceleration for MSBuild.Bootstrap

* Make EndToEndTests disposable

* Support running Analyzers.UnitTests from stage1

* Fix MSBuild.dll casing

* Don't run netfx Analyzer.UnitTests in Windows Core builds

* Fix Analyzers.UnitTests on Mac

* Renaming changes

* Renaming for clarity (#9754)

* Removing unnecessary types

* Code move

* Adjust namespaces

* Simplify TestEnvironments in EndToEndTests

* Support for per-project configuration, Acquisition mounting, etc

* Add more comments

* Grace handle double initialization attempts

* Fix tests

* Troubleshoot test, comment

* Reflect PR comments

* Fix build

* Fix build - proper multitargeting on core builds

* Adjust API naming and exposure

* Add forgotten acquisition data sending

* Renaming BuildCop to BuildCheck (#9893)

We decided to rename the analyzer project from BuildCop to BuildCheck. This PR is just reflecting that on the code.

* Renamed a few files missed (#9900)

* Add template for custom analyzers (#9789)

* Run tests against just-built bootstrap environment

* Reflecting PR comments

* Add test without analysis

* Rename tests project

* Force case renaming

* Simplify GlobalInstance initialization

* Remove multiple registrations checking

* Reflect on PR feedback

* Apply suggestions from code review

Co-authored-by: Mariana Dematte <[email protected]>

* Reflect on PR comments

* Update src/Build/BuildCheck/API/BuildAnalyzerRule.cs

Co-authored-by: Farhad Alizada <[email protected]>

* Reflect PR comments

---------

Co-authored-by: Ladi Prosek <[email protected]>
Co-authored-by: Mariana Dematte <[email protected]>
Co-authored-by: YuliiaKovalova <[email protected]>
Co-authored-by: Farhad Alizada <[email protected]>
JaynieBai added a commit that referenced this pull request May 27, 2024
Fixes #9915

Context
#9789 added a template for custom analyzer.
In order for that to be usable by our customers - we need to be publishing this to nuget feed along with our other binaries. So we should have the package produced from our repo - ideally as part of our build script

Changes Made
Create a template package project with property <GeneratePackageOnBuild>true</GeneratePackageOnBuild>that pack the templates into a nuget package when build. Add the package project in the msbuild solution in order to build the project.

Testing
https://dev.azure.com/devdiv/DevDiv/_build/results?buildId=9602998&view=artifacts&pathAsName=false&type=publishedArtifacts in the packageArtifacts folder.

image
JaynieBai added a commit that referenced this pull request Aug 6, 2024
…in the workflow (#10345)

Fixes #9923

Context
#9789 added a template for custom analyzer. Microsoft.Build version specified in the template should be synced with Version.props during packing. See https://github.com/dotnet/msbuild/pull/9789/files#r1521218723 for more details.
This needs to be done after #9915.

Changes Made
Create a workflow. When there are updates in the file Version.props, the flow will be trigger and check the version. If the version in the template is different form the VersionPrefix in the Version.props. It will create a PR to sync the version between them.
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.

5 participants