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

Sorting usings #923

Merged
merged 17 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from 15 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
2 changes: 1 addition & 1 deletion Shell/Release.psm1 → Shell/CreateRelease.psm1
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
function CSH-Release {
function CSH-CreateRelease {
param (
[Parameter(Mandatory=$true)]
[string]$versionNumber
Expand Down
12 changes: 12 additions & 0 deletions Shell/DeleteReviewBranches.psm1
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
function CSH-DeleteReviewBranches {

# TODO probably make this configurable
$pathToTestingRepo = "C:/Projects/csharpier-repos"
Set-Location $pathToTestingRepo

git checkout main
git branch | Where-Object { $_ -notmatch "main" } | ForEach-Object { git branch -D $_ }
git branch -r | Where-Object { $_ -notmatch "origin/main" } | ForEach-Object { git push origin --delete $_.Replace("origin/", "").Trim() }
}

Export-ModuleMember -Function CSH-*
2 changes: 1 addition & 1 deletion Shell/Init.ps1
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
foreach ($file in Get-ChildItem $PSScriptRoot -Filter "*.psm1") {
Import-Module $file.FullName -DisableNameChecking
Import-Module $file.FullName -DisableNameChecking -Force
}

Write-Host -ForegroundColor DarkMagenta "Welcome to CSharpier shell"
Expand Down
33 changes: 19 additions & 14 deletions Shell/ReviewBranch.psm1
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ function CSH-ReviewBranch {
$csharpierDllPath = Join-Path $repositoryRoot "Src/CSharpier.Cli/bin/release/net7.0/dotnet-csharpier.dll"

$location = Get-Location

Set-Location $repositoryRoot

if (!$pathToTestingRepo) {
Expand All @@ -32,36 +32,41 @@ function CSH-ReviewBranch {

if ($branch -eq "main") {
Write-Output "You must be on the branch you want to test. You are currently on main"
exit 1
return
}

$preBranch = "pre-" + $branch
$postBranch = "post-" + $branch

if ($folder -ne $null) {
$preBranch += "-" + $folder
$postBranch += "-" + $folder
}

Set-Location $pathToTestingRepo
& git reset --hard

git checkout $postBranch
$postBranchOutput = (git status) | Out-String
Set-Location $pathToTestingRepo
& git reset --hard *> $null
& git pull
try {
& git checkout $postBranch 2>&1
}
catch { }
$postBranchOutput = (git status 2>&1) | Out-String
$firstRun = -not $postBranchOutput.Contains("On branch $postBranch")

$fastParam = ""
if ($fast -eq $true) {
$fastParam = "--fast"
}

if ($firstRun)
{
if ($firstRun) {
Set-Location $repositoryRoot
$checkoutMainOutput = (git checkout main) | Out-String
if (-not $checkoutMainOutput.Contains("Your branch is up to date with ")) {
return
}
# try {
& git checkout main #2>&1 | Out-String
# }
# catch {
# Write-Host "Could not checkout main on csharpier, working directory is probably not clean"
# return
# }

CSH-BuildProject

Expand Down
27 changes: 21 additions & 6 deletions Src/CSharpier.FakeGenerators/SyntaxNodeComparerGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

namespace CSharpier.FakeGenerators;

using Microsoft.CodeAnalysis.CSharp.Syntax;

public class SyntaxNodeComparerGenerator
{
// this would probably be easier to understand as a scriban template but is a lot of effort
Expand Down Expand Up @@ -174,14 +176,27 @@ private static void GenerateMethod(StringBuilder sourceBuilder, Type type)
)
)
{
var compare = propertyType == typeof(SyntaxTokenList) ? "Compare" : "null";
if (propertyName == "Modifiers")
if (
propertyType.IsGenericType
&& propertyType.GenericTypeArguments[0] == typeof(UsingDirectiveSyntax)
)
{
propertyName += ".OrderBy(o => o.Text).ToList()";
sourceBuilder.AppendLine(
$" result = this.CompareUsingDirectives(originalNode.{propertyName}, formattedNode.{propertyName}, originalNode, formattedNode);"
);
}
sourceBuilder.AppendLine(
$" result = this.CompareLists(originalNode.{propertyName}, formattedNode.{propertyName}, {compare}, o => o.Span, originalNode.Span, formattedNode.Span);"
);
else
{
var compare = propertyType == typeof(SyntaxTokenList) ? "Compare" : "null";
if (propertyName == "Modifiers")
{
propertyName += ".OrderBy(o => o.Text).ToList()";
}
sourceBuilder.AppendLine(
$" result = this.CompareLists(originalNode.{propertyName}, formattedNode.{propertyName}, {compare}, o => o.Span, originalNode.Span, formattedNode.Span);"
);
}

sourceBuilder.AppendLine($" if (result.IsInvalid) return result;");
}
else if (
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,29 +1,13 @@
// leading
using First; // trailing

using A; // trailing
// leading with space
using // another trailing
Second;

using
// static leading
static // static trailing
Third;

using M = System.Math;
using Point = (int x, int y);

using static System.Math;

global using System;

using First;
using Second;
B;
Copy link
Collaborator

Choose a reason for hiding this comment

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

wtf is this?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I believe that was me going nuts trying comments everywhere a while back.
Apparently that is valid c#!


namespace Namespace
{
using Third;
using One.Two.Three;
using Third;

public class ClassName { }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
using System;
#if DEBUG
using Insite.Bad;
#endif
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#if DEBUG
using Insite.Bad;
#endif
using System;
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
using AWord;
using BWord;
using MWord;
using YWord;
using ZWord;
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
using MWord;
using ZWord;
using AWord;
using BWord;
using YWord;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
global using Global;
using System;
using global::Zebra;
using Custom;
using static Expression;
using Point = (int x, int y);
using Index = Microsoft.Framework.Index;
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#if BUILD_MSI_TASKS
using System;
using Microsoft.Build.Framework;

namespace RepoTasks;

class ClassName { }

#endif
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
using System.IO;
#if DEBUG
using System;
#else
using Microsoft;
#endif
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#if DEBUG
using System;
#else
using Microsoft;
#endif
using System.IO;
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
using System.IO;
#if !DEBUG
using System;
#else
using Microsoft;
#endif
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#if !DEBUG
using System;
#else
using Microsoft;
#endif
using System.IO;
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#if DEBUG
using A;
#else
using B;
#endif
#if !DEBUG
using C;
#endif
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
using C;
#if DEBUG
using A;
#else
using B;
#endif

#if !DEBUG
using C;
#endif
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#if DEBUG
using A;
#else
using B;
#endif
using C;
#if !DEBUG
using C;
#endif
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#if DEBUG
#define SYMBOL
#endif

using System;
using System.Collections;
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#if DEBUG
#define SYMBOL
#endif

using System.Collections;
using System;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#if DEBUG
extern alias MonoSecurity;
#else
using Mono.Security.Cryptography;
#endif

using System;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#if DEBUG
extern alias MonoSecurity;
#else
using Mono.Security.Cryptography;
#endif

using System;
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#if XMLCHARTYPE_GEN_RESOURCE
#undef XMLCHARTYPE_USE_RESOURCE
#endif

using System.Diagnostics;
using System.Threading;
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#if XMLCHARTYPE_GEN_RESOURCE
#undef XMLCHARTYPE_USE_RESOURCE
#endif

using System.Threading;
using System.Diagnostics;
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Licensed to something

using Apple;
using Microsoft;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there should be some test cases for code that does crazy things with comments, for example:

// Licensed to something

using Microsoft.AspNetCore;
// I like putting
using Microsoft;
/*
 * random comments everywhere
 */
using Apple;
/* because it's valid C# code */
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
// Look mom, I'm using a really fancy authentication library:
using Microsoft.Identity.Web.UI;
using Microsoft.Identity.Web; // another comment just for good measure

I'm unsure how this case should be handled, these are the options I could think of:

  • Keep everything as is, whoever put a comment between the using statements probably has a good reason (Maybe show a warning when formatting? I am not familiar with this codebase so i'm not sure if showing warnings is even possible)
  • Sort the individual blocks of usings between the comments
  • Be opinionated and just remove the comments, lol

As far as I can tell tell it's impossible to sort usings with comments while preserving the original intent of the comments in all cases.

I don't really care what the final solution is, just that it's tested and works with all the possible comment formats.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Keeping comments at the top was the only tricky part. Most comments are leading trivia which just goes with the node it is above. Trailing comments also stick with the node they are after. From all the code I reviewed, comments in usings aren't that common. #if is hard to deal with, but I have the basic cases covered. The .net analyzers themselves have some open issues around #ifs, so I'm not worried about the couple of edge cases that I couldn't resolve which appeared in the mono code.

// Licensed to something

/*
 * random comments everywhere
 */
using Apple;
// I like putting
using Microsoft;
using Microsoft.AspNetCore;
/* because it's valid C# code */
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
using Microsoft.Identity.Web; // another comment just for good measure
// Look mom, I'm using a really fancy authentication library:
using Microsoft.Identity.Web.UI;

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Licensed to something

using Microsoft;
using Apple;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
//

#define USE_STRUCT
using System;
using System.Runtime.CompilerServices;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
//

#define USE_STRUCT
using System.Runtime.CompilerServices;
using System;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
//

#undef USE_STRUCT
using System;
using System.Runtime.CompilerServices;
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
//

#undef USE_STRUCT
using System.Runtime.CompilerServices;
using System;

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
using A = Z;
using M = A;
using Z = M;
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
using System;
using System.Web;
using AWord;
using ZWord;
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
using ZWord;
using AWord;
using System.Web;
using System;
Loading
Loading