Add TargetFramework(s) unexpected check

documentation/specs/BuildCheck/Codes.md

@@ -10,7 +10,8 @@ Report codes are chosen to conform to suggested guidelines. Those guidelines are
 | [BC0104](#bc0104---projectreference-is-preferred-to-reference) | Warning | N/A | 9.0.200 | ProjectReference is preferred to Reference. |
 | [BC0105](#bc0105---embeddedresource-should-specify-culture-metadata) | Warning | N/A | 9.0.200 | Culture specific EmbeddedResource should specify Culture metadata. |
 | [BC0106](#bc0106---copytooutputdirectoryalways-should-be-avoided) | Warning | N/A | 9.0.200 | CopyToOutputDirectory='Always' should be avoided. |
-| [BC0107](#bc0107---targetframework-and-targetframeworks-specified-together) | Warning | N/A | 9.0.200 | TargetFramework and TargetFrameworks specified together. |
+| [BC0107](#bc0107---targetframework-and-targetframeworks-specified-together) | Warning | N/A | 9.0.200 | TargetFramework or TargetFrameworks specified in non-SDK style project. |
+| [BC0108](#bc0108---targetframework-or-targetframeworks-specified-in-non-sdk-style-project) | Warning | N/A | 9.0.300 | TargetFramework and TargetFrameworks specified together. |
 | [BC0201](#bc0201---usage-of-undefined-property) | Warning | Project | 9.0.100 | Usage of undefined property. |
 | [BC0202](#bc0202---property-first-declared-after-it-was-used) | Warning | Project | 9.0.100 | Property first declared after it was used. |
 | [BC0203](#bc0203----property-declared-but-never-used) | None | Project | 9.0.100 | Property declared but never used. |
@@ -126,6 +127,16 @@ If you specify `TargetFramework` you are instructing the build to produce a sing
 dotnet build my-multi-target.csproj /p:TargetFramework=net9.0
 ```
 
+<a name="BC0108"></a>
+## BC0108 - TargetFramework or TargetFrameworks specified in non-SDK style project.
+
+"'TargetFramework' nor 'TargetFrameworks' property should not be specified in non-SDK style projects."
+
+'TargetFramework' or 'TargetFrameworks' control the project output targets in modern .NET SDK-style projects. The older non-SDK style projects ('legacy style' projects) interprets different properties for similar mechanism (like 'TargetFrameworkVersion') and the 'TargetFramework' or 'TargetFrameworks' are silently ignored.
+
+Make sure the Target Framework targetting is done by properly understood mechanism.
+
+
 <a name="BC0201"></a>
 ## BC0201 - Usage of undefined property.
 

src/Build/BuildCheck/Checks/TargetFrameworkUnexpectedCheck.cs

@@ -0,0 +1,66 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Collections.Generic;
+using System.IO;
+using Microsoft.Build.Collections;
+using Microsoft.Build.Construction;
+using Microsoft.Build.Shared;
+
+namespace Microsoft.Build.Experimental.BuildCheck.Checks;
+internal class TargetFrameworkUnexpectedCheck : Check
+{
+    private const string RuleId = "BC0108";
+    public static CheckRule SupportedRule = new CheckRule(RuleId, "TargetFrameworkUnexpected",
+        ResourceUtilities.GetResourceString("BuildCheck_BC0108_Title")!,
+        ResourceUtilities.GetResourceString("BuildCheck_BC0108_MessageFmt")!,
+        new CheckConfiguration() { RuleId = RuleId, Severity = CheckResultSeverity.Warning });
+
+    public override string FriendlyName => "MSBuild.TargetFrameworkUnexpected";
+
+    public override IReadOnlyList<CheckRule> SupportedRules { get; } = [SupportedRule];
+
+    public override void Initialize(ConfigurationContext configurationContext)
+    {
+        /* This is it - no custom configuration */
+    }
+
+    public override void RegisterActions(IBuildCheckRegistrationContext registrationContext)
+    {
+        registrationContext.RegisterEvaluatedPropertiesAction(EvaluatedPropertiesAction);
+    }
+
+    internal override bool IsBuiltIn => true;
+
+    private readonly HashSet<string> _projectsSeen = new(MSBuildNameIgnoreCaseComparer.Default);
+
+    private void EvaluatedPropertiesAction(BuildCheckDataContext<EvaluatedPropertiesCheckData> context)
+    {
+        // We want to avoid repeated checking of a same project (as it might be evaluated multiple times)
+        //  for this reason we use a hashset with already seen projects.
+        if (!_projectsSeen.Add(context.Data.ProjectFilePath))
+        {
+            return;
+        }
+
+        string? frameworks = null;
+        string? framework = null;
+        // This is not SDK style project
+        if ((!context.Data.EvaluatedProperties.TryGetValue(PropertyNames.UsingMicrosoftNETSdk, out string? usingSdkStr) ||
+            !StringExtensions.IsMSBuildTrueString(usingSdkStr))
+            &&
+            // But TargetFramework(s) is specified
+            (context.Data.EvaluatedProperties.TryGetValue(PropertyNames.TargetFrameworks, out frameworks) ||
+            context.Data.EvaluatedProperties.TryGetValue(PropertyNames.TargetFramework, out framework)) &&
+            !string.IsNullOrEmpty(framework ?? frameworks))
+        {
+            // {0} specifies 'TargetFrameworks' property '{1}' and 'TargetFramework' property '{2}'
+            context.ReportResult(BuildCheckResult.Create(
+                SupportedRule,
+                // Populating precise location tracked via https://github.com/orgs/dotnet/projects/373/views/1?pane=issue&itemId=58661732
+                ElementLocation.EmptyLocation,
+                Path.GetFileName(context.Data.ProjectFilePath),
+                framework ?? frameworks ?? string.Empty));
+        }
+    }
+}

src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs

@@ -152,6 +152,7 @@ internal readonly record struct BuiltInCheckFactory(
                 new BuiltInCheckFactory([NoEnvironmentVariablePropertyCheck.SupportedRule.Id], NoEnvironmentVariablePropertyCheck.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<NoEnvironmentVariablePropertyCheck>),
                 new BuiltInCheckFactory([EmbeddedResourceCheck.SupportedRule.Id], EmbeddedResourceCheck.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<EmbeddedResourceCheck>),
                 new BuiltInCheckFactory([TargetFrameworkConfusionCheck.SupportedRule.Id], TargetFrameworkConfusionCheck.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<TargetFrameworkConfusionCheck>),
+                new BuiltInCheckFactory([TargetFrameworkUnexpectedCheck.SupportedRule.Id], TargetFrameworkUnexpectedCheck.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<TargetFrameworkUnexpectedCheck>),
             ],
 
             // BuildCheckDataSource.Execution

src/Build/Graph/ProjectInterpretation.cs

@@ -556,7 +556,7 @@ public bool RequiresTransitiveProjectReferences(ProjectGraphNode projectGraphNod
             // special case for Quickbuild which updates msbuild binaries independent of props/targets. Remove this when all QB repos will have
             // migrated to new enough Visual Studio versions whose Microsoft.Managed.After.Targets enable transitive references.
             if (string.IsNullOrWhiteSpace(projectInstance.GetEngineRequiredPropertyValue(AddTransitiveProjectReferencesInStaticGraphPropertyName)) &&
-                MSBuildStringIsTrue(projectInstance.GetEngineRequiredPropertyValue("UsingMicrosoftNETSdk")) &&
+                MSBuildStringIsTrue(projectInstance.GetEngineRequiredPropertyValue(PropertyNames.UsingMicrosoftNETSdk)) &&
                 MSBuildStringIsFalse(projectInstance.GetEngineRequiredPropertyValue("DisableTransitiveProjectReferences")))
             {
                 return true;

src/Build/Resources/Strings.resx

@@ -2198,6 +2198,14 @@ Utilization:          {0} Average Utilization: {1:###.0}</value>
     <value>Project {0} specifies 'TargetFrameworks' property '{1}' and 'TargetFramework' property '{2}' at the same time. This will lead to 'TargetFrameworks' being ignored and build will behave as single-targeted.</value>
     <comment>Terms in quotes are not to be translated.</comment>
   </data>
+  <data name="BuildCheck_BC0108_Title" xml:space="preserve">
+    <value>'TargetFramework' nor 'TargetFrameworks' property should not be specified in non-SDK style projects.</value>
+    <comment>Terms in quotes are not to be translated.</comment>
+  </data>
+  <data name="BuildCheck_BC0108_MessageFmt" xml:space="preserve">
+    <value>Project {0} specifies 'TargetFramework(s)' property '{1}', while it's not SDK-style project. Those properties are not understood by legacy-style projects and have no impact.</value>
+    <comment>Terms in quotes are not to be translated.</comment>
+  </data>
   <data name="BuildCheck_BC0201_Title" xml:space="preserve">
     <value>A property that is accessed should be declared first.</value>
   </data>