Don't move usings inside #if directives

This changes the move usings outside of a namespace rule to be very wary
of #if directives.  There is simply no safe way to move a directive of
the following nature:

``` csharp
namespace NS1
{
#if COND
using NS2;
class C { }
#else
using NS3;
class C { }
#endif
```

Lacking a good API to detect a using inside a #if directive we simply
abandon this rule if there are any #if directives in the file.

closes #71

Author: jaredpar

src/Microsoft.DotNet.CodeFormatting.Tests/Microsoft.DotNet.CodeFormatting.Tests.csproj

@@ -128,6 +128,7 @@
     <Compile Include="Rules\HasNewLineBeforeFirstUsingFormattingRuleTests.cs" />
     <Compile Include="Rules\PrivateFieldNamingRuleTests.cs" />
     <Compile Include="Rules\NonAsciiCharactersAreEscapedInLiteralsRuleTests.cs" />
+    <Compile Include="Rules\UsingLocationRuleTests.cs" />
   </ItemGroup>
   <ItemGroup>
     <Folder Include="Properties\" />

src/Microsoft.DotNet.CodeFormatting.Tests/Rules/UsingLocationRuleTests.cs

@@ -0,0 +1,132 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Xunit;
+
+namespace Microsoft.DotNet.CodeFormatting.Tests
+{
+    public sealed class UsingLocationRuleTests : SyntaxRuleTestBase
+    {
+        internal override ISyntaxFormattingRule Rule
+        {
+            get { return new Rules.UsingLocationRule(); }
+        }
+
+        [Fact]
+        public void SimpleMove()
+        {
+            var source = @"
+using NS1;
+namespace NS2
+{
+    using NS3;
+    class C1 { }
+}";
+
+            var expected = @"
+using NS1;
+using NS3;
+namespace NS2
+{
+    class C1 { }
+}";
+
+            Verify(source, expected);
+        }
+
+        [Fact]
+        public void SimpleMoveWithComment()
+        {
+            var source = @"
+using NS1;
+namespace NS2
+{
+    // test
+    using NS3;
+    class C1 { }
+}";
+
+            var expected = @"
+using NS1;
+// test
+using NS3;
+namespace NS2
+{
+    class C1 { }
+}";
+
+            Verify(source, expected);
+        }
+
+        [Fact]
+        public void SimpleMoveWithBeforeAfterComments()
+        {
+            var source = @"
+using NS1;
+namespace NS2
+{
+    // test1
+    using NS3;
+    // test2
+    class C1 { }
+}";
+
+            var expected = @"
+using NS1;
+// test1
+using NS3;
+namespace NS2
+{
+    // test2
+    class C1 { }
+}";
+
+            Verify(source, expected);
+        }
+
+        [Fact]
+        public void MoveToEmptyList()
+        {
+            var source = @"namespace NS2
+{
+    // test
+    using NS3;
+    class C1 { }
+}";
+
+            var expected = @"// test
+using NS3;
+namespace NS2
+{
+    class C1 { }
+}";
+
+            Verify(source, expected);
+        }
+
+        /// <summary>
+        /// In the case a using directive is inside of a #pragma directive there is no
+        /// way to safely move the using.
+        /// </summary>
+        [Fact]
+        public void Issue71()
+        {
+            var source = @"
+namespace N
+{
+#if false
+    using NS1;
+    class C { } 
+#else
+    using NS2;
+    using D { } 
+#endif
+}";
+
+            Verify(source, source);
+        }
+
+    }
+}

src/Microsoft.DotNet.CodeFormatting/Microsoft.DotNet.CodeFormatting.csproj

@@ -134,7 +134,7 @@
     <Compile Include="Rules\HasNoIllegalHeadersFormattingRule.cs" />
     <Compile Include="Rules\HasNoCustomCopyrightHeaderFormattingRule.cs" />
     <Compile Include="Rules\PrivateFieldNamingRule.cs" />
-    <Compile Include="Rules\HasUsingsOutsideOfNamespaceFormattingRule.cs" />
+    <Compile Include="Rules\UsingLocationRule.cs" />
     <Compile Include="Rules\FormatDocumentFormattingRule.cs" />
     <Compile Include="Rules\NonAsciiCharactersAreEscapedInLiteralsRule.cs" />
     <Compile Include="Rules\ExplicitThisRule.cs" />

src/Microsoft.DotNet.CodeFormatting/Rules/RuleOrder.cs

@@ -14,7 +14,7 @@ internal static class SyntaxRuleOrder
     {
         public const int HasNoCustomCopyrightHeaderFormattingRule = 1;
         public const int CopyrightHeaderRule = 2;
-        public const int HasUsingsOutsideOfNamespaceFormattingRule = 3;
+        public const int UsingLocationFormattingRule = 3;
         public const int HasNewLineBeforeFirstUsingFormattingRule = 4;
         public const int HasNewLineBeforeFirstNamespaceFormattingRule = 5;
         public const int BraceNewLineRule = 6;

src/Microsoft.DotNet.CodeFormatting/Rules/UsingLocationRule.cs

@@ -13,7 +13,10 @@
 
 namespace Microsoft.DotNet.CodeFormatting.Rules
 {
-    [SyntaxRuleOrder(SyntaxRuleOrder.HasUsingsOutsideOfNamespaceFormattingRule)]
+    /// <summary>
+    /// This will ensure that using directives are placed outside of the namespace.
+    /// </summary>
+    [SyntaxRuleOrder(SyntaxRuleOrder.UsingLocationFormattingRule)]
     internal sealed class UsingLocationRule : CSharpOnlyFormattingRule, ISyntaxFormattingRule
     {
         public SyntaxNode Process(SyntaxNode syntaxNode, string languageName)
@@ -22,50 +25,48 @@ public SyntaxNode Process(SyntaxNode syntaxNode, string languageName)
             if (root == null)
                 return syntaxNode;
 
-            var newRoot = root;
-            while (true)
+            // This rule can only be done safely as a syntax transformation when there is a single namespace
+            // declaration in the file.  Once there is more than one it opens up the possibility of introducing
+            // ambiguities to essentially make using directives global which were previously local.
+            var namespaceDeclarationList = root.Members.OfType<NamespaceDeclarationSyntax>().ToList();
+            if (namespaceDeclarationList.Count != 1)
             {
-                var namespaceWithUsings = newRoot.Members.OfType<NamespaceDeclarationSyntax>().FirstOrDefault(n => n.Usings.Any());
-                if (namespaceWithUsings == null)
-                    break;
-
-                // Moving a using with an alias out of a namespace is an operation which requires
-                // semantic knowledge to get correct.
-                if (namespaceWithUsings.Usings.Any(x => x.Alias != null))
-                    return syntaxNode;
-
-                // Remove nested usings
-
-                var emptyUsingList = SyntaxFactory.List<UsingDirectiveSyntax>();
-                var namespaceWithoutUsings = namespaceWithUsings.WithUsings(emptyUsingList);
-                newRoot = newRoot.ReplaceNode(namespaceWithUsings, namespaceWithoutUsings);
-
-                // Add usings to compilation unit
-
-                var usings = namespaceWithUsings.Usings.ToArray();
-
-                if (!newRoot.Usings.Any())
-                {
-                    // Specialize the case where there are no usings yet.
-                    //
-                    // We want to make sure that leading triviva becomes leading trivia
-                    // of the first using. 
-
-                    usings[0] = usings.First().WithLeadingTrivia(newRoot.GetLeadingTrivia());
-                    newRoot = newRoot.WithLeadingTrivia(Enumerable.Empty<SyntaxTrivia>());
+                return syntaxNode;
+            }
 
-                    // We want the last using to be separated from the namespace keyword
-                    // by a blank line.
+            var namespaceDeclaration = namespaceDeclarationList.Single();
+            var usingList = namespaceDeclaration.Usings;
+            if (usingList.Count == 0)
+            {
+                return syntaxNode;
+            }
 
-                    var trailingTrivia = usings.Last().GetTrailingTrivia();
-                    var linebreak = new[] { SyntaxFactory.CarriageReturnLineFeed };
-                    usings[usings.Length - 1] = usings.Last().WithTrailingTrivia(trailingTrivia.Concat(linebreak));
-                }
+            // Moving a using with an alias out of a namespace is an operation which requires
+            // semantic knowledge to get correct.
+            if (usingList.Any(x => x.Alias != null))
+            {
+                return syntaxNode;
+            }
 
-                newRoot = newRoot.AddUsings(usings);
+            // We don't have the capability to safely move usings which are embedded inside an #if
+            // directive.  
+            //
+            //  #if COND
+            //  using NS1;
+            //  #endif
+            //
+            // At the time there isn't a great way (that we know of) for detecting this particular 
+            // case.  Instead we simply don't do this rewrite if the file contains any #if directives.
+            if (root.DescendantTrivia().Any(x => x.Kind() == SyntaxKind.IfDirectiveTrivia))
+            {
+                return syntaxNode;
             }
 
+            var newRoot = root;
+            newRoot = newRoot.ReplaceNode(namespaceDeclaration, namespaceDeclaration.WithUsings(SyntaxFactory.List<UsingDirectiveSyntax>()));
+            newRoot = newRoot.WithUsings(newRoot.Usings.AddRange(namespaceDeclaration.Usings));
+
             return newRoot;
         }
     }
-}
+}