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

Generate error message for duplicate items #624

Merged
merged 4 commits into from
Jan 13, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
50 changes: 50 additions & 0 deletions src/Tasks/Microsoft.NET.Build.Tasks/CheckForDuplicateItems.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
using Microsoft.Build.Framework;
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Text;

namespace Microsoft.NET.Build.Tasks
{
public class CheckForDuplicateItems : TaskBase
{
[Required]
public ITaskItem [] Items { get; set; }

[Required]
public string ItemName { get; set; }

public bool DefaultItemsEnabled { get; set; }

public bool DefaultItemsOfThisTypeEnabled { get; set; }

[Required]
public string PropertyNameToDisableDefaultItems { get; set; }

public string PropertyValueToDisableDefaultItems { get; set; } = "false";

[Required]
public string MoreInformationLink { get; set; }

protected override void ExecuteCore()
{
if (DefaultItemsEnabled && DefaultItemsOfThisTypeEnabled)
{
var duplicateItems = Items.GroupBy(i => i.ItemSpec).Where(g => g.Count() > 1).ToList();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can required inputs be null? Change to

(Items ?? Enumerable<ITaskItem>.Empty())

if (duplicateItems.Any())
{
string duplicateItemsFormatted = string.Join("; ", duplicateItems.Select(d => $"'{d.Key}'"));

string message = string.Format(CultureInfo.CurrentCulture, Strings.DuplicateItemsError,
ItemName,
PropertyNameToDisableDefaultItems,
PropertyValueToDisableDefaultItems,
duplicateItemsFormatted);

Log.LogError(message);
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
</PackageReference>
</ItemGroup>
<ItemGroup>
<Compile Include="CheckForDuplicateItems.cs" />
<Compile Include="DiagnosticMessageSeverity.cs" />
<Compile Include="DiagnosticsHelper.cs" />
<Compile Include="NETSdkError.cs" />
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Tasks/Microsoft.NET.Build.Tasks/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -225,4 +225,7 @@
<data name="NU1012" xml:space="preserve">
<value>Dependency conflict. '{0}' expected '{1}' but received '{2}'</value>
</data>
<data name="DuplicateItemsError" xml:space="preserve">
<value>Duplicate '{0}' items were included. The .NET SDK includes '{0}' items from your project directory by default. You can either remove these items from your project file, or set the '{1}' property to '{2}' if you want to explicitly include them in your project file. The duplicate items were: {3}</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,45 @@ Copyright (c) .NET Foundation. All rights reserved.
<Content Remove="$(DefaultRemoves)" />
</ItemGroup>

<UsingTask TaskName="CheckForDuplicateItems" AssemblyFile="$(MicrosoftNETBuildTasksAssembly)" />

<Target Name="CheckForDuplicateItems" BeforeTargets="_CheckForInvalidConfigurationAndPlatform">

<PropertyGroup>
<!-- TODO: Create a fwlink for this -->
<DefaultItemsMoreInformationLink>https://github.com/dotnet/sdk</DefaultItemsMoreInformationLink>
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to have an acceptable link before we merge.


<DefaultPackageReferencesEnabled Condition="'$(DisableImplicitFrameworkReferences)' != 'true'">true</DefaultPackageReferencesEnabled>
</PropertyGroup>

<CheckForDuplicateItems
Items="@(Compile)"
ItemName="Compile"
DefaultItemsEnabled="$(EnableDefaultItems)"
DefaultItemsOfThisTypeEnabled="$(EnableDefaultCompileItems)"
PropertyNameToDisableDefaultItems="EnableDefaultCompileItems"
MoreInformationLink="$(DefaultItemsMoreInformationLink)"
/>

<CheckForDuplicateItems
Items="@(EmbeddedResource)"
ItemName="EmbeddedResource"
DefaultItemsEnabled="$(EnableDefaultItems)"
DefaultItemsOfThisTypeEnabled="$(EnableDefaultEmbeddedResourceItems)"
PropertyNameToDisableDefaultItems="EnableDefaultEmbeddedResourceItems"
MoreInformationLink="$(DefaultItemsMoreInformationLink)"
/>

<!-- TODO: Do we check for duplicate content items here, or in the Web SDK? EnableDefaultContentItems isn't defined or used at all by the .NET SDK -->
Copy link
Member

Choose a reason for hiding this comment

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

I'd say to do it here because Content is a type known in Common.targets, and this is the closest layer above that. If it were defined and used only in Web, I'd say to move it there.

<CheckForDuplicateItems
Items="@(Content)"
ItemName="Content"
DefaultItemsEnabled="$(EnableDefaultItems)"
DefaultItemsOfThisTypeEnabled="$(EnableDefaultContentItems)"
PropertyNameToDisableDefaultItems="EnableDefaultContentItems"
MoreInformationLink="$(DefaultItemsMoreInformationLink)"
/>

</Target>

</Project>
77 changes: 77 additions & 0 deletions test/Microsoft.NET.Build.Tests/GivenThereAreDefaultItems.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using System.Xml.Linq;
using Xunit;
using static Microsoft.NET.TestFramework.Commands.MSBuildTest;
using Microsoft.NET.TestFramework.ProjectConstruction;

namespace Microsoft.NET.Build.Tests
{
Expand Down Expand Up @@ -352,6 +353,82 @@ public void Compile_items_can_be_explicitly_specified_while_default_EmbeddedReso
GivenThatWeWantAllResourcesInSatellite.TestSatelliteResources(_testAssetsManager, projectChanges, setup, "ExplicitCompileDefaultEmbeddedResource");
}

[Fact]
public void It_gives_an_error_message_if_duplicate_compile_items_are_included()
{
var testProject = new TestProject()
{
Name = "DuplicateCompileItems",
TargetFrameworks = "netstandard1.6",
IsSdkProject = true
};

var testAsset = _testAssetsManager.CreateTestProject(testProject)
.WithProjectChanges(project =>
{
var ns = project.Root.Name.Namespace;
var itemGroup = new XElement(ns + "ItemGroup");
project.Root.Add(itemGroup);
itemGroup.Add(new XElement(ns + "Compile", new XAttribute("Include", @"**\*.cs")));
})
.Restore(testProject.Name);

var buildCommand = new BuildCommand(Stage0MSBuild, Path.Combine(testAsset.TestRoot, testProject.Name));

WriteFile(Path.Combine(buildCommand.ProjectRootPath, "Class1.cs"), "public class Class1 {}");

buildCommand
.CaptureStdOut()
.Execute()
.Should()
.Fail()
.And.HaveStdOutContaining("DuplicateCompileItems.cs")
.And.HaveStdOutContaining("Class1.cs")
.And.HaveStdOutContaining("EnableDefaultCompileItems");
}

[Fact]
public void It_gives_the_correct_error_if_duplicate_compile_items_are_included_and_default_items_are_disabled()
{
var testProject = new TestProject()
{
Name = "DuplicateCompileItems",
TargetFrameworks = "netstandard1.6",
IsSdkProject = true
};

var testAsset = _testAssetsManager.CreateTestProject(testProject, "DuplicateCompileItemsWithDefaultItemsDisabled")
.WithProjectChanges(project =>
{
var ns = project.Root.Name.Namespace;

project.Root.Element(ns + "PropertyGroup").Add(
new XElement(ns + "EnableDefaultCompileItems", "false"));

var itemGroup = new XElement(ns + "ItemGroup");
project.Root.Add(itemGroup);
itemGroup.Add(new XElement(ns + "Compile", new XAttribute("Include", @"**\*.cs")));
itemGroup.Add(new XElement(ns + "Compile", new XAttribute("Include", @"DuplicateCompileItems.cs")));
})
.Restore(testProject.Name);

var buildCommand = new BuildCommand(Stage0MSBuild, Path.Combine(testAsset.TestRoot, testProject.Name));

WriteFile(Path.Combine(buildCommand.ProjectRootPath, "Class1.cs"), "public class Class1 {}");

buildCommand
.CaptureStdOut()
.Execute()
.Should()
.Fail()
.And.HaveStdOutContaining("DuplicateCompileItems.cs")
// Class1.cs wasn't included multiple times, so it shouldn't be mentioned
.And.NotHaveStdOutMatching("Class1.cs")
// Default items weren't enabled, so the error message should come from the C# compiler and shouldn't include the information about default compile items
.And.HaveStdOutContaining("MSB3105")
.And.NotHaveStdOutMatching("EnableDefaultCompileItems");
}

void RemoveGeneratedCompileItems(List<string> compileItems)
{
// Remove auto-generated compile items.
Expand Down
3 changes: 1 addition & 2 deletions test/Microsoft.NET.TestFramework/TestAsset.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ internal void FindProjectFiles()
{
_projectFiles = new List<string>();

var files = Directory.GetFiles(base.Path, "*.*", SearchOption.AllDirectories)
.Where(file => !IsInBinOrObjFolder(file));
var files = Directory.GetFiles(base.Path, "*.*", SearchOption.AllDirectories);

foreach (string file in files)
{
Expand Down