HasUsingsOutsideNamespace rule can introduce warning CS0105

[nslottow]

namespace A
{
    using System;
}

namespace B
{
    using System;
}

results in

using System;

using System;
namespace A
{
}

namespace B
{
}

warning CS0105: The using directive for 'System' appeared previously in this namespace

See nslottow/codeformatter@ef3bbba

[krwq]

We should probably add a rule which cleans up usings

[AlexGhiondea]

We should do that, but we should also catch this case when we promote usings.

[krwq]

if we clean them up later then we can remove duplicates. I don't think there is a purpose in doing it twice

[AlexGhiondea]

I think a rule should not make the code worse. What if the rule to clean-up the usings is not run? That will leave the code with a warning.

I think we can code this rule in such a way that we don't introduce a warning.

[jaredpar]

Properly implementing this rule requires some level of semantic analysis.

CC'd @jasonmalinowski in case he knows of an API that could help us here.

[jasonmalinowski]

We don't have any public cleanup API for this, just the building blocks.

Don't forget that promotion itself requires semantics: if you have "using C" inside a namespace A it might either bind to A.C or to global::C, and you don't know which. Also, using aliases can include unqualified types that can't be moved to the top level. Moving a declaration might also change binding and break other code if there's now an ambiguous type. This is very hard to get right. Unless we have a major codebase right now that's breaking the rule and we need and automated tool to fix it, I'd say just flag it, don't try to fix it, and have the developer do it for now.

[bartonjs]

Some instances of this seem detectable, like having the rule say "if the exact clause I'm adding already exists, just ignore it". HashSet and sort the output to just rewrite the whole section.

This seemed particularly interesting on a file just now:

// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using global::System;
using global::System.Diagnostics;
using global::System.Security.Cryptography;

namespace System.Security.Cryptography
{
    using global::System;
    using global::System.Diagnostics;
    using global::System.Security.Cryptography;
    public partial interface ICspAsymmetricAlgorithm
    {
        CspKeyContainerInfo CspKeyContainerInfo { get;}
        byte[] ExportCspBlob(bool includePrivateParameters);
        void ImportCspBlob(byte[] rawData);
    }
}

The difference being "but, it was already there..." instead of "oh, that's funny, you moved it twice."

(And in this case it's particularly sad, because none of the types were resolved via usings, they were all "in the current namespace" or keywords)