Update Reference and ProjectReference ordering in our targets.

Switch the to using a common DefaultReference notion between targets
and eliminated targetingpack.props. Moved the default references for
tests into tests.targets.

Fixed where we added Private to References because in some cases it wasn't happing
because the target that set Private=true was run before all the references were
added. This is why we are now setting Reference Private=false in a different target
from ProjectReference Private=false because they sometimes need to happen at different
times in the target execution.

Set _FindDependencies=false to block RAR from trying to automatically
add the closure of the primary references as it added automatic references that folks
could accidently start depending on and we want to be explicit about our dependencies.
This was really noticable once we started auto referencing the netstandard facade which
caused everything to get pulled into the reference set.

To fix the various build scenarios where we add ProjectReferences automatically we needed
a common target to hook on which we now have AddProjectReferencesDynamically target that
correctly hooks into the various dependency chains. See comment in targets file for more
details.

10 files changed, 81 insertions, 69 deletions

diff --git a/Tools-Override/FrameworkTargeting.targets b/Tools-Override/FrameworkTargeting.targets
index 21f9f35e9a..dcb87fe7ac 100644
--- a/Tools-Override/FrameworkTargeting.targets
+++ b/Tools-Override/FrameworkTargeting.targets
@@ -1,17 +1,6 @@
 <?xml version="1.0" encoding="utf-8"?>
 <Project ToolsVersion="12.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
 
-  <PropertyGroup Condition="'$(IncludeDefaultReferences)' == ''">
-    <IncludeDefaultReferences Condition="'$(MSBuildProjectExtension)' == '.csproj'">true</IncludeDefaultReferences>
-    <IncludeDefaultReferences Condition="'$(MSBuildProjectExtension)' == '.vbproj'">true</IncludeDefaultReferences>
-  </PropertyGroup>
-
-  <ItemGroup>
-    <!-- netstandard is a default reference whenever building for NETStandard or building an implementation assembly -->
-    <DefaultReference Condition="$(NuGetTargetMoniker.StartsWith('.NETStandard')) OR '$(IsReferenceAssembly)' != 'true'"
-                      Include="netstandard" />
-  </ItemGroup>
-
   <ItemGroup>
     <TargetingPackDirs Include="$(RefPath)" />
     <AdditionalReferencePaths Include="@(TargetingPackDirs)" />
@@ -21,6 +10,8 @@
     <ContractOutputPath>$(RefPath)</ContractOutputPath>
     <FrameworkPathOverride>$(ContractOutputPath)</FrameworkPathOverride>
     <AssemblySearchPaths>$(AssemblySearchPaths);$(ContractOutputPath);{RawFileName}</AssemblySearchPaths>
+    <!-- Disable RAR from transitively discovering depdencies for References -->
+    <_FindDependencies>false</_FindDependencies>
   </PropertyGroup>
 
   <PropertyGroup Condition=" '$(TargetFrameworkIdentifier)' == ''
@@ -63,27 +54,32 @@
     <NuGetTargetMoniker Condition="'$(NuGetTargetMoniker)' == ''">.NETCoreApp,Version=v1.0</NuGetTargetMoniker>
   </PropertyGroup>
 
-  <Target Name="AddDefaultReferences"
-          Condition="'$(IncludeDefaultReferences)' =='true'"
-          BeforeTargets="BeforeResolveReferences">
-    <ItemGroup>
-      <!-- Include default references when specified and they exist -->
-      <Reference Condition="Exists('$(RefPath)%(Identity).dll')" Include="@(DefaultReference)" />
+  <PropertyGroup Condition="'$(IncludeDefaultReferences)' == ''">
+    <IncludeDefaultReferences Condition="'$(MSBuildProjectExtension)' == '.csproj'">true</IncludeDefaultReferences>
+    <IncludeDefaultReferences Condition="'$(MSBuildProjectExtension)' == '.vbproj'">true</IncludeDefaultReferences>
+  </PropertyGroup>
+
+  <Target Name="SetupDefaultReferences">
+    <ItemGroup Condition="'$(IncludeDefaultReferences)' =='true'">
+      <!-- netstandard is a default reference whenever building for NETStandard or building an implementation assembly -->
+      <DefaultReference Condition="$(NuGetTargetMoniker.StartsWith('.NETStandard')) OR '$(IsReferenceAssembly)' != 'true'"
+                        Include="netstandard" />
     </ItemGroup>
   </Target>
 
-  <Target Name="MarkReferencePrivateFalse"
-          BeforeTargets="ResolveAssemblyReferences;AssignProjectConfiguration">
+  <Target Name="UpdateReferenceItems"
+          DependsOnTargets="SetupDefaultReferences"
+          BeforeTargets="BeforeResolveReferences"
+  >
+    <ItemGroup>
+      <Reference Include="@(DefaultReference)" />
+    </ItemGroup>
+
     <ItemGroup>
       <!-- Simple name references will be resolved from the targeting pack folders and should never be copied to output -->
       <Reference Condition="'%(Reference.Extension)' != '.dll'">
         <Private>false</Private>
       </Reference>
-
-      <!-- Project references for non-test assemblies should never be copied to the output. -->
-      <ProjectReference Condition="'$(IsTestProject)' != 'true'">
-        <Private>false</Private>
-      </ProjectReference>
     </ItemGroup>
   </Target>
 
@@ -127,6 +123,28 @@
   <Import Project="$(MSBuildToolsPath)\Microsoft.VisualBasic.targets"
           Condition="'$(TargetFrameworkIdentifier)' != '.NETPortable' and '$(MSBuildProjectExtension)' == '.vbproj'" />
 
+  <PropertyGroup>
+    <ResolveReferencesDependsOn>
+      AddProjectReferencesDynamically;
+      $(ResolveReferencesDependsOn);
+    </ResolveReferencesDependsOn>
+    <CleanDependsOn>
+      AddProjectReferencesDynamically;
+      $(CleanDependsOn);
+    </CleanDependsOn>
+  </PropertyGroup>
+  <!--
+    Common targets don't provide a good place to enable adding new ProjectReference items in targets that work
+    with both clean, build, and rebuild entry point targets. We cannot hook off of AssignProjectConfigurations
+    because it is conditioned on "'@(ProjectReference)'!=''" which gets evalulated before the BeforeTargets run
+    so adding ProjectReference as part of a BeforeTarget make still have the AssignProjectConfiguration skipped.
+    To help with this problem we are creating a new target and correctly hooking it up in the resolve and clean
+    depends on target chains.
+
+    For information on evaulation of targets ordering see https://msdn.microsoft.com/en-us/library/ee216359.aspx.
+  -->
+  <Target Name="AddProjectReferencesDynamically" DependsOnTargets="$(AddProjectReferencesDynamicallyDependsOn)" />
+
   <PropertyGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETFramework' and '$(OutputType)' == 'exe'">
     <!-- RAR thinks all EXEs require binding redirects.  That's not the case for CoreCLR -->
     <AutoUnifyAssemblyReferences>true</AutoUnifyAssemblyReferences>
diff --git a/Tools-Override/resolveContract.targets b/Tools-Override/resolveContract.targets
index 9a76265298..f8f98711f1 100644
--- a/Tools-Override/resolveContract.targets
+++ b/Tools-Override/resolveContract.targets
@@ -2,15 +2,14 @@
 <Project ToolsVersion="12.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
 
   <PropertyGroup Condition="'$(ResolveMatchingContract)' == 'true'">
-    <ResolveReferencesDependsOn>
+    <AddProjectReferencesDynamicallyDependsOn>
       ResolveMatchingContract;
+      $(AddProjectReferencesDynamicallyDependsOn);
+    </AddProjectReferencesDynamicallyDependsOn>
+    <ResolveReferencesDependsOn>
       $(ResolveReferencesDependsOn);
       VerifyMatchingContract
     </ResolveReferencesDependsOn>
-    <CleanDependsOn>
-      ResolveMatchingContract;
-      $(CleanDependsOn);
-    </CleanDependsOn>
   </PropertyGroup>
 
   <Target Name="ResolveMatchingContract">
diff --git a/Tools-Override/tests.targets b/Tools-Override/tests.targets
index 068a31a252..0e926536ab 100644
--- a/Tools-Override/tests.targets
+++ b/Tools-Override/tests.targets
@@ -99,6 +99,25 @@
     </ItemGroup>
   </Target>
 
+  <Target Name="AddDefaultTestReferences" BeforeTargets="SetupDefaultReferences">
+    <ItemGroup Condition="'$(IsTestProject)'=='true' and '$(IncludeDefaultReferences)' == 'true'">
+      <TargetingPackExclusions Include="System.Runtime.WindowsRuntime.UI.Xaml" /> <!-- Harmless, but causes PRI targets to run -->
+      <TargetingPackExclusions Include="@(ReferenceFromRuntime)"/>
+
+      <!-- Whitelisted runtime assemblies that are OK to reference. -->
+      <ReferenceFromRuntime Include="xunit.core" />
+      <ReferenceFromRuntime Include="Xunit.NetCore.Extensions" />
+      <ReferenceFromRuntime Include="xunit.assert" />
+      <ReferenceFromRuntime Include="xunit.abstractions" />
+      <ReferenceFromRuntime Include="xunit.performance.core" />
+
+      <!-- Reference everything in the targeting pack directory -->
+      <TargetingPackItems Include="%(TargetingPackDirs.Identity)/*.dll" />
+
+      <DefaultReference Include="%(TargetingPackItems.Filename)" Exclude="@(TargetingPackExclusions)" />
+    </ItemGroup>
+  </Target>
+
   <!-- Generate the script to run the tests.  The script performs two high-level steps:
        1.  Copies the common test runtime dependencies calculated in DiscoverTestDependencies to the test
            execution directory. Each copy command no-ops if the file already exists in the test execution
diff --git a/buildvertical.targets b/buildvertical.targets
index 3c841f0a7e..891061899d 100644
--- a/buildvertical.targets
+++ b/buildvertical.targets
@@ -27,7 +27,7 @@
       <_projectBuildConfigurations>
         <AdditionalProperties>Configuration=%(Identity);%(_projectBuildConfigurations.AdditionalProperties)</AdditionalProperties>
       </_projectBuildConfigurations>
-      
+
       <!-- transform back to project -->
       <_projectWithConfiguration Include="@(_projectBuildConfigurations->'%(OriginalItemSpec)')" />
     </ItemGroup>
@@ -83,7 +83,7 @@
 
   <!-- Runs in a leaf project (csproj) to determine best configuration for ProjectReferences -->
   <Target Name="AnnotateProjectReference"
-          BeforeTargets="BeforeResolveReferences"
+          BeforeTargets="AssignProjectConfiguration"
           Condition="'@(ProjectReference)' != ''"
           Inputs="%(ProjectReference.Identity)"
           Outputs="unused">
@@ -116,6 +116,11 @@
       <ProjectReference>
         <AdditionalProperties>Configuration=$(_projectReferenceConfiguration);%(ProjectReference.AdditionalProperties)</AdditionalProperties>
       </ProjectReference>
+
+      <!-- Project references for non-test assemblies should never be copied to the output. -->
+      <ProjectReference Condition="'$(IsTestProject)' != 'true'">
+        <Private>false</Private>
+      </ProjectReference>
     </ItemGroup>
   </Target>
 
diff --git a/dir.props b/dir.props
index b4a3ae00b0..d18b2c5562 100644
--- a/dir.props
+++ b/dir.props
@@ -54,8 +54,6 @@
     <BuildToolsTargetsDesktop Condition="'$(RunningOnCore)' != 'true'">true</BuildToolsTargetsDesktop>
   </PropertyGroup>
 
-  <Import Project="$(MSBuildThisFileDirectory)targetingpacks.props" />
-
   <!-- Enable the analyzers for this repo -->
   <PropertyGroup>
     <EnableDotnetAnalyzers Condition="'$(EnableDotnetAnalyzers)'==''">true</EnableDotnetAnalyzers>
diff --git a/referenceFromRuntime.targets b/referenceFromRuntime.targets
index 216157862b..5abd924aa5 100644
--- a/referenceFromRuntime.targets
+++ b/referenceFromRuntime.targets
@@ -9,8 +9,8 @@
     </ItemGroup>
   </Target>
 
-  <Target Name="AddRuntimeProjectReference" 
-          BeforeTargets="AnnotateProjectReference"
+  <Target Name="AddRuntimeProjectReference"
+          BeforeTargets="AddProjectReferencesDynamically"
           Condition="'$(IsTestProject)'!='true' AND '@(ReferenceFromRuntime)' != ''">
     <Error Condition="'$(IsReferenceAssembly)' != 'true' AND '$(RuntimeProjectFile)' == ''" Text="RuntimeProjectFile must be specified when using ReferenceFromRuntime from source projects." />
     <Error Condition="'$(IsReferenceAssembly)' == 'true'" Text="ReferenceFromRuntime may not be used from reference assemblies." />
@@ -22,8 +22,8 @@
       </ProjectReference>
     </ItemGroup>
   </Target>
-  
-  <Target Name="FilterReferenceFromRuntime" 
+
+  <Target Name="FilterReferenceFromRuntime"
           AfterTargets="ResolveProjectReferences"
           Condition="'$(IsTestProject)'!='true' AND '@(ReferenceFromRuntime)' != ''">
     <ItemGroup>
diff --git a/src/System.Linq.Expressions/tests/System.Linq.Expressions.Tests.csproj b/src/System.Linq.Expressions/tests/System.Linq.Expressions.Tests.csproj
index 59cefeb5fb..e9c670df12 100644
--- a/src/System.Linq.Expressions/tests/System.Linq.Expressions.Tests.csproj
+++ b/src/System.Linq.Expressions/tests/System.Linq.Expressions.Tests.csproj
@@ -253,5 +253,8 @@
     <Compile Include="ILReader\LocalsSignatureParser.cs" />
     <Compile Include="ILReader\SigParser.cs" />
   </ItemGroup>
+  <ItemGroup>
+    <ReferenceFromRuntime Include="xunit.execution.dotnet" />
+  </ItemGroup>
   <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
 </Project>
\ No newline at end of file
diff --git a/src/System.Linq.Expressions/tests/TestExtensions/TestOrderer.cs b/src/System.Linq.Expressions/tests/TestExtensions/TestOrderer.cs
index 35985108d8..5168d7733a 100644
--- a/src/System.Linq.Expressions/tests/TestExtensions/TestOrderer.cs
+++ b/src/System.Linq.Expressions/tests/TestExtensions/TestOrderer.cs
@@ -16,7 +16,7 @@ namespace System.Linq.Expressions.Tests
             Dictionary<int, List<TTestCase>> queue = new Dictionary<int, List<TTestCase>>();
             foreach (TTestCase testCase in testCases)
             {
-                Xunit.Abstractions.IAttributeInfo orderAttribute = ReflectionAbstractionExtensions.GetCustomAttributes(testCase.TestMethod.Method, typeof(TestOrderAttribute)).FirstOrDefault();
+                Xunit.Abstractions.IAttributeInfo orderAttribute = testCase.TestMethod.Method.GetCustomAttributes(typeof(TestOrderAttribute)).FirstOrDefault();
                 int order;
                 if (orderAttribute == null || (order = orderAttribute.GetConstructorArguments().Cast<int>().First()) == 0)
                 {
diff --git a/src/System.Runtime.Serialization.Json/tests/Performance/System.Runtime.Serialization.Json.Performance.Tests.csproj b/src/System.Runtime.Serialization.Json/tests/Performance/System.Runtime.Serialization.Json.Performance.Tests.csproj
index 83e7f2739b..623c621b5e 100644
--- a/src/System.Runtime.Serialization.Json/tests/Performance/System.Runtime.Serialization.Json.Performance.Tests.csproj
+++ b/src/System.Runtime.Serialization.Json/tests/Performance/System.Runtime.Serialization.Json.Performance.Tests.csproj
@@ -16,7 +16,7 @@
     <Compile Include="JsonNetPerformanceTest.cs" />
   </ItemGroup>
   <ItemGroup>
-    <None Include="project.json" />
+    <ReferenceFromRuntime Include="Newtonsoft.Json" />
   </ItemGroup>
   <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
 </Project>
\ No newline at end of file
diff --git a/targetingpacks.props b/targetingpacks.props
deleted file mode 100644
index 9f4df8b4e7..0000000000
--- a/targetingpacks.props
+++ /dev/null
@@ -1,30 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>
-<Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
-
-  <!-- Adds test references to the targeting pack and runtime assemblies. -->
-  <Target Name="AddTestTargetingPackReferences" BeforeTargets="ResolveAssemblyReferences">
-    <ItemGroup Condition="'$(IsTestProject)'=='true'">
-      <TargetingPackExclusions Include="System.Private.CoreLib" />
-      <TargetingPackExclusions Include="System.Runtime.WindowsRuntime.UI.Xaml" /> <!-- Harmless, but causes PRI targets to run -->
-      <TargetingPackExclusions Include="@(ReferenceFromRuntime)"/>
-
-      <!-- Whitelisted runtime assemblies that are OK to reference. -->
-      <ReferenceFromRuntime Include="xunit.core" />
-      <ReferenceFromRuntime Include="Xunit.NetCore.Extensions" />
-      <ReferenceFromRuntime Include="xunit.assert" />
-      <ReferenceFromRuntime Include="xunit.abstractions" />
-      <ReferenceFromRuntime Include="xunit.performance.core" />
-      <ReferenceFromRuntime Include="xunit.execution.dotnet" Condition="'$(_bc_TargetGroup)'!='netfx'"/>
-      <ReferenceFromRuntime Include="Newtonsoft.Json" Condition="'$(SkipIncludeNewtonsoftJson)'!='true'" />
-
-      <!-- Add Reference's to all files in the targeting pack folder, and to whitelisted items from the group above in the runtime folder. -->
-      <TargetingPackItems Include="%(TargetingPackDirs.Identity)/*" Condition="'$(NoTargetingPackReferences)' != 'true'" />
-    </ItemGroup>
-
-    <ItemGroup>
-      <Reference Include="%(TargetingPackItems.Filename)" Exclude="@(TargetingPackExclusions)"> <!-- TODO: System.Private.CoreLib shouldn't even be in the targeting pack. -->
-        <Private>false</Private>
-      </Reference>
-    </ItemGroup>
-  </Target>
-</Project>