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.

Sign up for GitHub

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

Jump to bottom

Only run code style analyzers when nullable warnings are enabled #43972

Merged
merged 1 commit into from
May 6, 2020
Merged

Only run code style analyzers when nullable warnings are enabled #43972

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions eng/targets/Imports.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@
to only set in the inner builds, not the outer build where only $(TargetFrameworks) is defined.
-->
<DisableNullableWarnings Condition="'$(DisableNullableWarnings)' == '' AND $(TargetFrameworks.Contains('netcoreapp3.1')) AND '$(TargetFramework)' != '' AND '$(TargetFramework)' != 'netcoreapp3.1'">true</DisableNullableWarnings>

<!--
Disable code style analyzers in "older" targets for a multi-targeted project. These analyzers don't
impact the correctness of the output, so we avoid the performance overhead where it's easy to do so.
-->
<RoslynCheckCodeStyle Condition="'$(RoslynCheckCodeStyle)' == '' AND '$(DisableNullableWarnings)' == 'true'">false</RoslynCheckCodeStyle>
<RoslynCheckCodeStyle Condition="'$(RoslynCheckCodeStyle)' == '' AND ('$(ContinuousIntegrationBuild)' != 'true' OR '$(RoslynEnforceCodeStyle)' == 'true')">true</RoslynCheckCodeStyle>
</PropertyGroup>

<PropertyGroup Condition="'$(DisableNullableWarnings)' == 'true'">
Expand Down
9 changes: 4 additions & 5 deletions eng/targets/Settings.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
<DisableImplicitNuGetFallbackFolder>true</DisableImplicitNuGetFallbackFolder>
<ToolsetPackagesDir>$(RepoRoot)build\ToolsetPackages\</ToolsetPackagesDir>
<RoslynPortableTargetFrameworks>netcoreapp3.1;net472</RoslynPortableTargetFrameworks>
<RoslynCheckCodeStyle Condition="'$(ContinuousIntegrationBuild)' != 'true' or '$(RoslynEnforceCodeStyle)' == 'true'">true</RoslynCheckCodeStyle>
<UseSharedCompilation>true</UseSharedCompilation>

<Features>strict, IOperation</Features>
Expand Down Expand Up @@ -149,14 +148,14 @@

<!-- Language-specific analyzer packages -->
<Choose>
<When Condition="'$(Language)' == 'VB' and '$(RoslynCheckCodeStyle)' == 'true'">
<ItemGroup>
<When Condition="'$(Language)' == 'VB'">
<ItemGroup Condition="'$(RoslynCheckCodeStyle)' == 'true'">
Copy link
Member

Choose a reason for hiding this comment

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

The property this is conditioning on is defined in the targets file now, not the props. Think we should move this <Choose> to the targets file as well.

Copy link
Member Author

@sharwell sharwell May 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

All property groups are evaluated before all item groups. The SDK itself has many examples of cases where default properties are assigned in the targets file (allowing a project file to set values before the defaults are applied as fallback), and items in the props file (allowing a project file to Update one or more items after the item exists).

<PackageReference Include="Microsoft.CodeAnalysis.VisualBasic.CodeStyle" Version="$(MicrosoftCodeAnalysisVisualBasicCodeStyleVersion)" PrivateAssets="all" />
</ItemGroup>
</When>

<When Condition="'$(Language)' == 'C#' and '$(RoslynCheckCodeStyle)' == 'true'">
<ItemGroup>
<When Condition="'$(Language)' == 'C#'">
<ItemGroup Condition="'$(RoslynCheckCodeStyle)' == 'true'">
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.CodeStyle" Version="$(MicrosoftCodeAnalysisCSharpCodeStyleVersion)" PrivateAssets="all" />
</ItemGroup>
</When>
Expand Down