Skip to content

Commit

Permalink
Add, Update and Format build Comments
Browse files Browse the repository at this point in the history
- Remove redundant comments.
- Add missing comments on certain code blocks.
- Format code in comments with proper quote style.
  • Loading branch information
Nirmal4G committed Jan 2, 2023
1 parent 4271041 commit 8e6a805
Show file tree
Hide file tree
Showing 12 changed files with 63 additions and 21 deletions.
5 changes: 5 additions & 0 deletions Directory.Build.targets
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

<Import Project="$(BuildToolsDirectory)Toolkit.Common.targets" />

<!--
The Git Version task from `Nerdbank.GitVersioning` overwrites the `InformationalVersion`
that was supposed to hold the `SourceRevisionId`. Hence, Add the `SourceRevisionId`
into Assembly Metadata with the `CommitHash` key.
-->
<Target Name="AddCommitHashToAssemblyAttributes" BeforeTargets="GetAssemblyAttributes">
<ItemGroup>
<AssemblyAttribute Include="System.Reflection.AssemblyMetadataAttribute" Condition="'$(SourceRevisionId)' != ''">
Expand Down
6 changes: 3 additions & 3 deletions eng/Toolkit.Common.props
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
</PropertyGroup>

<!--
Suppress ref safety warnings in unsafe contexts (see https://github.com/dotnet/csharplang/issues/6476).
This is used eg. to replace Unsafe.SizeOf<T>() calls with just sizeof(T). The warning is not necessary
since in order to use these APIs the caller already has to be in an unsafe context.
Suppress `ref` usage safety warnings in unsafe contexts (see https://github.com/dotnet/csharplang/issues/6476).
This is used to replace `Unsafe.*` calls with standard language notations (e.g., `Unsafe.SizeOf<T>()` to `sizeof(T)`).
The `CS8500` warning is not necessary, since in order to use these APIs the caller already has to be in an unsafe context.
-->
<PropertyGroup>
<NoWarn>$(NoWarn);CS8500</NoWarn>
Expand Down
1 change: 0 additions & 1 deletion eng/Toolkit.Common.targets
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
</PropertyGroup>

<PropertyGroup Condition="'$(IsPackable)' == 'true'">
<!-- TODO: Dynamically generate Title if one wasn't set -->
<Title Condition="'$(Title)' == ''">$(Product) Asset</Title>
<PackageTags Condition="'$(PackageTags)' != ''">$(CommonTags);$(PackageTags)</PackageTags>
<PackageTags Condition="'$(PackageTags)' == ''">$(CommonTags)</PackageTags>
Expand Down
4 changes: 3 additions & 1 deletion eng/Toolkit.TextTemplates.targets
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
<Project>

<!-- T4 service used by the Guard APIs -->
<!-- T4 text template service used by the Visual Studio project system -->
<ItemGroup>
<Service Include="{508349b6-6b84-4df5-91f0-309beebad82d}" />
</ItemGroup>

<!-- Using T4 text templates to generate sources -->
<ItemGroup>
<None Update="**\*.tt">
<Generator>TextTemplatingFileGenerator</Generator>
<LastGenOutput>%(Filename).g.cs</LastGenOutput>
</None>
</ItemGroup>

<!-- Sources generated based on the above text templates -->
<ItemGroup>
<Compile Update="**\*.g.cs">
<AutoGen>True</AutoGen>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@
</When>
</Choose>

<!--
Adds support for generating sources using T4 text templating engine.
The Guard APIs in 'Generated' folder is generated using text templates.
-->
<Import Project="$(BuildToolsDirectory)Toolkit.TextTemplates.targets" />

</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<TargetFramework>netstandard2.0</TargetFramework>
</PropertyGroup>

<!-- Sources that are used by Generator logic in the target projects -->
<ItemGroup>
<Compile Remove="EmbeddedResources\*.cs" />
<EmbeddedResource Include="EmbeddedResources\*.cs">
Expand All @@ -14,6 +15,10 @@
</EmbeddedResource>
</ItemGroup>

<!--
These files enables tracking Analyzer releases for the rules that ship from this project.
See https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md for more info.
-->
<ItemGroup>
<AdditionalFiles Include="AnalyzerReleases.Shipped.md" />
<AdditionalFiles Include="AnalyzerReleases.Unshipped.md" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<TargetFramework>netstandard2.0</TargetFramework>
</PropertyGroup>

<!-- Sources that are used by Generator logic in the target projects -->
<ItemGroup>
<Compile Remove="EmbeddedResources\*.cs" />
<EmbeddedResource Include="EmbeddedResources\*.cs">
Expand All @@ -14,6 +15,10 @@
</EmbeddedResource>
</ItemGroup>

<!--
These files enables tracking Analyzer releases for the rules that ship from this project.
See https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md for more info.
-->
<ItemGroup>
<AdditionalFiles Include="AnalyzerReleases.Shipped.md" />
<AdditionalFiles Include="AnalyzerReleases.Unshipped.md" />
Expand Down
14 changes: 10 additions & 4 deletions src/CommunityToolkit.Mvvm/CommunityToolkit.Mvvm.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@
</ItemGroup>

<!--
Include the custom .targets file to check the source generator.
.NET 6 is not needed as it guarantees Roslyn 4.x.
Include a custom targets file to check the compiler version required for the source generator.
It needs Roslyn 4.x and including the targets in .NET 6+ is not needed as it guarantees the same.
-->
<ItemGroup>
<None Include="CommunityToolkit.Mvvm.targets" Pack="True" PackagePath="build\netstandard2.0" />
Expand Down Expand Up @@ -74,8 +74,14 @@
</ItemGroup>

<!--
Pack the source generator to the right package folders (each matching the target Roslyn version).
Roslyn will automatically load the highest version compatible with Roslyn's version in the SDK.
Pack the source generator build outputs to the correct package folders (for each target Roslyn version)
for them to be used as analyzers. Roslyn compilers will automatically load the highest version compatible
with the Roslyn's version in the SDK.
HACK: The recommended way to pack is to get the build outputs using the project's built-in targets,
and include them using NuGet's Pack targets. However, due to a bug in v5 of those targets,
the outputs were not included in the package. So, including them directly via final
output path and building it first is the only way to ensure they are included.
-->
<ItemGroup>
<None Include="..\CommunityToolkit.Mvvm.SourceGenerators\bin\$(Configuration)\netstandard2.0-roslyn4.0\CommunityToolkit.Mvvm.SourceGenerators.dll" PackagePath="analyzers\dotnet\roslyn4.0\cs" Pack="true" Visible="false" />
Expand Down
29 changes: 17 additions & 12 deletions src/CommunityToolkit.Mvvm/CommunityToolkit.Mvvm.targets
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<Project>

<!-- Get the analyzer from the CommunityToolkit.Mvvm NuGet package -->
<!-- Get the analyzer from the 'CommunityToolkit.Mvvm' NuGet package -->
<Target Name="MVVMToolkit_GatherAnalyzers">
<ItemGroup>
<MVVMToolkit_Analyzer Include="@(Analyzer)" Condition="'%(Analyzer.NuGetPackageId)' == 'CommunityToolkit.Mvvm'" />
Expand All @@ -14,20 +14,20 @@
DependsOnTargets="MVVMToolkit_GatherAnalyzers">

<!--
Use the CSharpCoreTargetsPath property to find the version of the compiler we are using. This is the same mechanism
Use the 'CSharpCoreTargetsPath' property to find the version of the compiler we are using. This is the same mechanism
MSBuild uses to find the compiler. We could check the assembly version for any compiler assembly (since they all have
the same version) but Microsoft.Build.Tasks.CodeAnalysis.dll is where MSBuild loads the compiler tasks from so if
the same version) but 'Microsoft.Build.Tasks.CodeAnalysis.dll' is where MSBuild loads the compiler tasks from so if
someone is getting creative with msbuild tasks/targets this is the "most correct" assembly to check.
-->
<GetAssemblyIdentity AssemblyFiles="$([System.IO.Path]::Combine($([System.IO.Path]::GetDirectoryName($(CSharpCoreTargetsPath))), 'Microsoft.Build.Tasks.CodeAnalysis.dll'))">
<Output TaskParameter="Assemblies" ItemName="MVVMToolkit_CurrentCompilerAssemblyIdentity"/>
</GetAssemblyIdentity>

<PropertyGroup>
<!-- Transform the resulting item from GetAssemblyIdentity into a property representing its assembly version -->
<!-- Transform the resulting item from 'GetAssemblyIdentity' task into a property representing its assembly version -->
<MVVMToolkit_CurrentCompilerVersion>@(MVVMToolkit_CurrentCompilerAssemblyIdentity->%(Version))</MVVMToolkit_CurrentCompilerVersion>

<!-- The CurrentCompilerIsNotNewEnough property can now be defined based on the Roslyn assembly version -->
<!-- The 'MVVMToolkit_CurrentCompilerIsNotNewEnough' property can now be defined based on the Roslyn assembly version -->
<MVVMToolkit_CurrentCompilerIsNotNewEnough Condition="$([MSBuild]::VersionLessThan($(MVVMToolkit_CurrentCompilerVersion), 4.0))">true</MVVMToolkit_CurrentCompilerIsNotNewEnough>
</PropertyGroup>

Expand All @@ -45,7 +45,7 @@
</Target>

<!--
Manually remove additional analyzers if Roslyn component versioning is not supported (ie. if a legacy .csproj project is used).
Manually remove additional analyzers if Roslyn component versioning is not supported (i.e., if a legacy .csproj project is used).
This target is only run if Roslyn 4.0 or greater is present, as otherwise all analyzers would have already been removed anyway.
-->
<Target Name="MVVMToolkit_RemoveAdditionalAnalyzers_WhenRoslynComponentVersioningIsNotSupported"
Expand All @@ -63,18 +63,23 @@
</PropertyGroup>

<!--
This condition is a bit convoluted, but it's essentially just selecting all analyzers from the NuGet package that don't have the target Roslyn directory name in their full path.
For instance, if Roslyn 4.3 is the highest supported version, the target directory name will be "roslyn 4.3", and this condition will filter out all analyzers with a path such
as: "C:\...\.nuget\...\CommunityToolkit.Mvvm\analyzers\roslyn4.0\cs\CommunityToolkit.Mvvm". The [System.String]::Copy trick is used to achieve two things: we can't directly
invoke a property function (ie. Contains in this case) on a metadata item, so we need an intermediate string to invoke it on. We could also use [System.String]::new, but using
Copy is more efficient (on .NET Framework runtime) as it'll just skip the allocation entirely.
The following line is just removing all the analyzers present in the NuGet package from the `Analyzer` item list
that doesn't have the target Roslyn identifier as the folder name in their full path.
For Example:
If Roslyn 4.3 is the highest supported version, the Roslyn identifier (aka folder name) will be `roslyn4.3`, and
this condition will remove all analyzers with a path such as: "[CommunityToolkit.Mvvm]\analyzers\roslyn4.0\cs\*.dll".
Here, We can't directly invoke a property function (ie. `Contains` in this case) on a metadata item,
so we need an intermediate string to invoke it on. We could have used `[System.String]::new()` method,
but it allocates in the .NET Framework runtime. Instead, we use `[System.String]::Copy` which doesn't allocate.
-->
<ItemGroup>
<Analyzer Remove="@(MVVMToolkit_Analyzer)" Condition="!$([System.String]::Copy(%(MVVMToolkit_Analyzer.FullPath)).Contains($(MVVMToolkit_SelectedRoslynAnalyzerTarget)))"/>
</ItemGroup>
</Target>

<!-- Remove the analyzer if Roslyn is missing -->
<!-- Remove the analyzer if Roslyn is missing or the project is not using C# -->
<Target Name="MVVMToolkit_RemoveAnalyzers_WhenRoslynIsNotFound"
Condition="'$(CSharpCoreTargetsPath)' == ''"
AfterTargets="ResolvePackageDependenciesForBuild;ResolveNuGetPackageAssets"
Expand Down
1 change: 1 addition & 0 deletions src/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

<Import Project="..\$(MSBuildThisFile)" />

<!-- Common properties for all projects under src folder -->
<PropertyGroup>
<IsPackable>true</IsPackable>
<IsPublishable>true</IsPublishable>
Expand Down
4 changes: 4 additions & 0 deletions src/Directory.Build.targets
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,16 @@

<Import Project="..\$(MSBuildThisFile)" />

<!-- Configure defaults for projects targeting .NET 6 and above -->
<PropertyGroup Condition="'$(_NET_6_OR_GREATER)' == 'true'">
<!-- Enable trimming on supported targets -->
<IsTrimmable>true</IsTrimmable>
<EnableTrimAnalyzer>true</EnableTrimAnalyzer>
<!-- Define `NETSTANDARD2_1_OR_GREATER` to include .NET Standard 2.1 shared APIs -->
<DefineConstants>NETSTANDARD2_1_OR_GREATER</DefineConstants>
</PropertyGroup>

<!-- Common Assembly and Module attributes across projects -->
<ItemGroup>
<Compile Include="$(BuildToolsDirectory)AssemblyInfo.Shared.cs" LinkBase="Properties" Visible="False" />
</ItemGroup>
Expand Down
6 changes: 6 additions & 0 deletions tests/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

<Import Project="..\$(MSBuildThisFile)" />

<!--
By default, all projects under tests are neither packed nor published.
This also includes non-test projects as well. But if it's required then
override the following properties only within those projects.
-->
<PropertyGroup>
<IsPackable>false</IsPackable>
<IsPublishable>false</IsPublishable>
Expand All @@ -10,6 +15,7 @@

<PropertyGroup>
<IsTestProject>true</IsTestProject>
<!-- The MVVM Toolkit unit tests use `Mvvm.ExternalAssembly` project as a `ProjectReference` for testing -->
<IsTestProject Condition="$(MSBuildProjectName.Contains('Mvvm.ExternalAssembly'))">false</IsTestProject>
</PropertyGroup>

Expand Down

0 comments on commit 8e6a805

Please sign in to comment.