Simplified Property Groups

[mhutch]

It would be nice to support a terser and more readable syntax for PropertyGroup.

By making Properties an alias for PropertyGroup and allowing properties to be provided as attributes instead / in addition to child elements, this:

<PropertyGroup>
    <Foo>A</Foo>
    <Bar>B</Bar>
</PropertyGroup>

could be simplified to

<Properties
    Foo="A"
    Bar="B"
/>

This is a pretty big readability improvement.

It would also be nice to alias Items to ItemGroup.

[NickCraver]

I'd pitch a few more aliases here, for example <References> and <Includes>. In line with this issue: though they'd have no net effect, they'd make the human editable form much more readable.

Unrelated to this but along the same vein, I think something like <References Framework='net45'> would also increase readability. How much of a work tradeoff are we looking at for aliasing vs. elements with additional attribute support?

/cc @davkean

As an example, here's a file today vs. with proposed changes (I've inlined the Version attribute for comparison on both as this is already planned):

<Project ToolsVersion="15.0">
  <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" />
  <PropertyGroup>
    <TargetFrameworks>net45;netstandard1.5</TargetFrameworks>
    <AssemblyName>MiniProfiler.Shared</AssemblyName>
    <OutputType>Library</OutputType>
    <OutputPath>bin\</OutputPath>
    <SignAssembly>true</SignAssembly>
    <AssemblyOriginatorKeyFile>..\MiniProfiler\miniprofiler.snk</AssemblyOriginatorKeyFile>
  </PropertyGroup>
  <ItemGroup>
    <Compile Include="**\*.cs" />
    <EmbeddedResource Include="**\*.resx" />
  </ItemGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Sdk" Version="1.0.0-alpha-20161104-2" PrivateAssets="All"/>
    <PackageReference Include="NETStandard.Library" Version="1.6.1"/>
  </ItemGroup>
  <ItemGroup Condition="'$(TargetFramework)' == 'netstandard1.5'">
    <PackageReference Include="System.ComponentModel.Primitives" Version="4.3.0"/>
    <PackageReference Include="System.Data.Common" Version="4.3.0"/>
    <PackageReference Include="System.Diagnostics.StackTrace" Version="4.3.0"/>
    <PackageReference Include="System.Dynamic.Runtime" Version="4.3.0"/>
    <PackageReference Include="System.Reflection.Emit.Lightweight" Version="4.3.0"/>
    <PackageReference Include="System.Runtime.Extensions" Version="4.3.0"/>
    <PackageReference Include="System.Runtime.Serialization.Primitives" Version="4.3.0"/>
    <PackageReference Include="System.Threading.Tasks.Parallel" Version="4.3.0"/>
    <PackageReference Include="Newtonsoft.Json" Version="9.0.1"/>
  </ItemGroup>
  <ItemGroup Condition="'$(TargetFramework)' == 'net45'">
    <Reference Include="Microsoft.CSharp" />
    <Reference Include="System" />
    <Reference Include="System.Configuration" />
    <Reference Include="System.Data" />
    <Reference Include="System.Data.Linq" />
    <Reference Include="System.Runtime.Serialization" />
    <Reference Include="System.Transactions" />
    <Reference Include="System.Web" />
    <Reference Include="System.Web.Extensions" />
    <Reference Include="System.Xml" />
  </ItemGroup>
  <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
</Project>

Proposed:

<Project ToolsVersion="15.0">
  <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" />
  <Properties>
    <TargetFrameworks>net45;netstandard1.5</TargetFrameworks>
    <AssemblyName>MiniProfiler.Shared</AssemblyName>
    <OutputType>Library</OutputType>
    <OutputPath>bin\</OutputPath>
    <SignAssembly>true</SignAssembly>
    <AssemblyOriginatorKeyFile>..\MiniProfiler\miniprofiler.snk</AssemblyOriginatorKeyFile>
  </Properties>
  <Includes>
    <Compile Include="**\*.cs" />
    <EmbeddedResource Include="**\*.resx" />
  </Includes>
  <References>
    <PackageReference Include="Microsoft.NET.Sdk" Version="1.0.0-alpha-20161104-2" PrivateAssets="All"/>
    <PackageReference Include="NETStandard.Library" Version="1.6.1"/>
  </References>
  <References Framework="netstandard1.5">
    <PackageReference Include="System.ComponentModel.Primitives" Version="4.3.0"/>
    <PackageReference Include="System.Data.Common" Version="4.3.0"/>
    <PackageReference Include="System.Diagnostics.StackTrace" Version="4.3.0"/>
    <PackageReference Include="System.Dynamic.Runtime" Version="4.3.0"/>
    <PackageReference Include="System.Reflection.Emit.Lightweight" Version="4.3.0"/>
    <PackageReference Include="System.Runtime.Extensions" Version="4.3.0"/>
    <PackageReference Include="System.Runtime.Serialization.Primitives" Version="4.3.0"/>
    <PackageReference Include="System.Threading.Tasks.Parallel" Version="4.3.0"/>
    <PackageReference Include="Newtonsoft.Json" Version="9.0.1"/>
  </References>
  <References Framework="net45">
    <Reference Include="Microsoft.CSharp" />
    <Reference Include="System" />
    <Reference Include="System.Configuration" />
    <Reference Include="System.Data" />
    <Reference Include="System.Data.Linq" />
    <Reference Include="System.Runtime.Serialization" />
    <Reference Include="System.Transactions" />
    <Reference Include="System.Web" />
    <Reference Include="System.Web.Extensions" />
    <Reference Include="System.Xml" />
  </References>
  <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
</Project>

[davkean]

One thought I had was that instead of MSBuild having special knowledge of individual properties, such as "Framework" (which it doesn't do today), it would instead simply allow a simpler condition syntax.

Therefore something like this:

<ItemGroup TargetFramework="net45">
...
</ItemGroup>

Would be treated as if I wrote:

<ItemGroup Condition="'$(TargetFramework)'=='net45">

<ItemGroup>

Then you could do something like:

<PropertyGroup TargetFramework="net45" Configuration="Debug">
<!-- Only applies when targeting .NET 4.5 under Debug -->
</PropertyGroup>

Or:

<ItemGroup>
    <!-- Only reference when targeting UWP -->
    <ProjectReference Include="UWPSupport.csproj" TargetFramework="UWP" />
</ItemGroup>

[NickCraver]

@davkean That's an extremely attractive option - what's the thinking on existing attributes and collision with property names with this layout? Known attributes would be preferred? I could see that, but then if you didn't know it was an attribute would be fairly confusing. I wonder if we could figure out a way to indicate it's a conditional and bypass the conflict case.

Or am I over-analyzing this?

[davkean]

Not over analyzing this, there is a concern. Especially that item metadata has been prompted to attributes -even more likely that there will be clashes. I'll let the MSBuild chime into the issues - maybe you could make it less likely by only allowing the syntax on "groups"; <ItemGroup>, <PropertyGroup>.

[gulbanana]

that option looks like a large readability improvement, even if only on groups

[dasMulli]

An additional readability improvement would be to to alias ItemGroup to Items and PropertyGroup to Properties. I'm guessing the name originated from a code element that was named to represent a collection..

For readability, this would be a small change but a huge improvement:

<Properties>
    <Awesome>true</Awesome>
</Properties>
<Items>
    <Compile Include="..." />
</Items>

If i understand msbuild correctly, there could be an issue if there was a task named like that when adding properties or items inside of a task.

[NickCraver]

@dasMulli I'm confused...that was the original issue. Are you talking about other changes, or just adding <Items> to the list?

@mhutch Perhaps an update up top (if you agree with these) with an overall alias list would be helpful?

[dasMulli]

@NickCraver slightly different: The original proposal of the issue is adding the properties as attributes, which is a more substantial change than "just" aliasing.

[mhutch]

Added the Items alias.

I think the simplified conditions could work on itemgroups, properties, targets, tasks etc. But on propertygroups it would directly collide with my proposal, and on items there would be a very high risk of collision with metadata attributes. So... I'm not sure what I think about it.

[mhutch]

Also, check out #821

[rainersigwald]

My thoughts on PropertyGroup to Properties (and ItemGroup to Items) aliasing: I wish that had been the original name, but I don't see enough benefit from enabling it now. We'd have to allow the old names and the new names, making parsing and persisting projects after OM manipulation more complicated both for us and for the folks that treat MSBuild projects as straight XML (that I wish didn't happen but does).