belav / csharpier Goto Github PK

Find some other complex examples + look into why this one isn't breaking. Possibly the ForceFlat for string interpolation is preventing it from breaking.

writeLine(hasArgs ? $"            return this.{apiType}<{effectiveType}>(\"{query.name}\", parameters, selectsBuilder);" : $"            return this.{apiType}<{effectiveType}>(\"{query.name}\", null, selectsBuilder);");

This should break somewhere

foreach(var someReallyLongNameYeah in this.SomeOtherReallyLongNameToGetStuff()) {
    break;
}

Start with some smaller repos
mission-control
stonemason
beacon

Some bigger ones
insite-commerce
newtonsoft.json

Etc

BaseList for a class currently formats like this. It should indent AndYetAnotherLongClassName

public class LongClassNameBreak
    : AnotherLongClassName<T>,
    AndYetAnotherLongClassName
{
    public void MethodName() { }
}

--validate - to validate formatting did not appear to lose source. This could also format twice to ensure formatting won't be flip blopping
--write - to actually write files to disk this seems unnecessary
[directory] - to determine where to format files. Better syntax and ignoring files/directories in Beta
--check - validate no files need formatting - probably not til Beta
--parallel - format files in parallel - probably not til Beta
--quiet - don't write out info about each file unless there are issues - probably not til Beta, let's stick to just outputting information about failures for now.

See https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1413.md for why they should be forced.

This is how it currently breaks

[Obsolete, System.NonSerialized, NonSerialized, CLSCompliant(true
|| false
& true)]
private volatile int f2;

It should break something like this

[
    Obsolete, 
    System.NonSerialized, 
    NonSerialized, 
    CLSCompliant(true || false true),
]
private volatile int f2;

The extra line before the brace is inserted if there is a trailing comma

            return new LeadingComment
            {
                Type = commentType,
                Comment = comment,
                AddExtraIndent = addExtraIndent,

            };

A side effect of #22 is that now extra lines are kept in situations where we would probably want to get rid of them.

    public 
    // random comment
    
    // other random comment
    void Method() { }

        if (
            someCondition
            // random comment

            // other random comment
            || someOtherCondition
        )
        {
            return;
        }

This feels like an edge case that probably won't come up very often.

#40 may be somewhat related

I noticed that I don't get consistent total file counts while I was testing something today.
Is it really not formatting the same number of files, or is the count of files not being updated correctly?

Take this example

var longNameForcesLineBreakYeahLongName =
    someValue == someOtherValue && x == 7;

The && is the root BinaryExpression, which is a LogicalAnd
The left and right of it are EqualsExpression.
CSharpier should group the LogicalAnd, but if it groups every binary Expression than the following doesn't format correctly.

if (
    longStatementName
    && longerStatementName
    && evenLongerStatementName
    && superLongStatementName
) { }

Which is basically a tree of BinaryExpressions where the Left is LogicalAnd BinaryExpression and the Right is IdentifierName.
This may require flattening out BinaryExpressions and conditionally grouping them based on what operation they are performing.

See prettier/src/language-js/print/binaryish.js for all the logic that goes into making this work for js

if (someBoolean)
{
// code using value
}

The comment is leading trivia on the closing brace. Currently leading trivia ends up indented at the same level as the token it is on.
Because we are changing line breaks/indenting we can't really depend on the existing whitespace in front of the comment.
This probably needs to be handled as a special case somehow. There may be other special cases like this.

What options are there currently for getting csharpier to run in an IDE?
Could plugins for the IDE's be developed?

Currently when an if statement has a long expression, it formats like

if (
    !this.model.TranslatableFields.TryGetValue(
        fieldName,
        out var dictionary
    )
)
{
    return;
}

I think it should do this

if (
    !this.model.TranslatableFields.TryGetValue(
        fieldName,
        out var dictionary
    )
) {
    return;
}

if (shortStatement)
{
    return;
}

or

if (
    !this.model.TranslatableFields.TryGetValue(
        fieldName,
        out var dictionary
    ))
{
    return;
}

if (shortStatement)
{
    return;
}

This applies to other syntax nodes, like for loops and while loops.

This may have less to do with the property, and more to do with the syntax inside of the lamda body

public Language CurrentLanguage =>
    this.currentLanguage ?? (this.currentLanguage = this.UnitOfWork.GetRepository<Language>()
        .Get(SiteContext.Current.LanguageDto.Id));

Should be something more like

public Language CurrentLanguage =>
    this.currentLanguage ?? 
        (this.currentLanguage = this.UnitOfWork.GetRepository<Language>().Get(
            SiteContext.Current.LanguageDto.Id
        ));

Given the following IfDirective, roslyn does not parse the contents because DEBUG is not defined

#if DEBUG
        Console.WriteLine("Rosyln does not parse this because DEBUG is not defined");
#endif

The contents are just a block of text.
Can I determine some way to parse those contents and format them? Currently they are just written out as is.

Add support for a --check option.
This can be used to validate that files are already formatted but will not make any changes to files.

Publish from new branch?
Or start using branches for new work?
Or kick off publish against master and continue to work in master?

The following inserts a new line after the trailing trivia on every format.

class ClassName
{
    void Method()
    {
        var x = 1; /* no newline after this */
        var y = 2;
    }
}

Some examples of property formatting that needs work

        public virtual ICollection<AdminActionPermission> ActionPermissions
        { get; set;
        } = new HashSet<AdminActionPermission>();

        public Dictionary<string, string> Properties { get; set; } = new Dictionary<string,
            string>();

        public virtual PropertyConfiguration PropertyConfiguration
        { get; set;
        }

Also review repositories for other property issues.

If you start running csharpier and ctrl-c the process in the middle it will sometimes leave a file half written, resulting in lost code and no indication that the code was lost.

Run unit tests, and if they pass
Then run what is in publish.txt

I believe there are a couple very common syntax nodes that lack any kind of line breaks/indenting. Review a couple repos to look for some of these.

It currently uses recursion + reflection and could probably be a lot more efficient.
I wrote a quick generated version that got rid of the reflection, but it ran into stack overflows. I should redo that without recursion.

Initial ideas

1. width
2. tabs vs spaces
3. configurable number of spaces
4. force braces
5. maybe brace style
6. maybe trailing commas
7. maybe end of line, \n vs \r\n

Read https://prettier.io/docs/en/option-philosophy.html and prettier/prettier#40 again before adding any options.

Things to document

1. How to install
2. CLI
3. How to use in IDES - maybe
4. What does alpha mean
5. What comes next

Long declarations don't break, and declarations with initializers don't really break in a clean way.

    private string SomeFieldWithSadsfasdfasdfasdfuperLongNameYeahMan = "kljasdfkljasdklfkjlasdfkjaskdlfasasdfasdfasdfasdfdfksdf";

    private static readonly Regex FrontEndResourceRelativePathRegex = new Regex(
        @".*?/Themes/.*?/.*?/(.*)",
        RegexOptions.Compiled |
        RegexOptions.CultureInvariant |
        RegexOptions.IgnoreCase);

    private SomeObject SomeLongerName = new SomeObject(
        "lkjasdflkjasdfkljaskldjf",
        "klasldkfaksdfasdfkjasdklf") { Property1 = 1, Property2 = 2 };

When there are options available for formatting - #10 - it should be possible to specify those options in a config file. That will ensure that if csharpier is run via command line, build task, or some other way, it will use the same set of options without having to duplicate them.

This may be cleaned up by InvocationExpression. Do that formatting first.

            using (var http = new HttpClient())
            using (var stream = Assembly.GetExecutingAssembly().GetManifestResourceStream(
                "GraphQLApiGenerator.MetadataQuery.gql"))
            using (var queryReader = new StreamReader(stream))
            using (var content = new ByteArrayContent(
                Encoding.UTF8.GetBytes(
                    JsonConvert.SerializeObject(
                        new { query = await queryReader.ReadToEndAsync() }))))
            {

There are several files in the commerce repo that fail to compile after formatting. After digging in, they are all originally USC-2 LE BOM encoding. After they are saved, they no longer have an encoding.

I added a line to specifically exclude the file by name to make sure the csharpier --check validation will work on PRs
This appears to have something to do with leading or trailing trivia

Currently formats like

public static implicit operator string(Person p) => p.First + " " + p.Last;

But should be like

public static implicit operator string(Person p)
    => p.First + " " + p.Last;

Lamda body methods break this way already and I thought operators and methods uses the same formatting code.

See AttributeLists.cst

[BigLongAttributeWhatHappensHmmmmmmmmmmmmmmmmmmmmmmmmmmmmm(
        "11111111111111111111111111",
        "22222222222222222222222222")]
void LongAttribute() { }
// should be
[BigLongAttributeWhatHappensHmmmmmmmmmmmmmmmmmmmmmmmmmmmmm(
    "11111111111111111111111111",
    "22222222222222222222222222")]
void LongAttribute() { }

The following are CSharpSyntaxNodes that do not currently have their own print method. I know some of these are handled by a parent method and cannot exist on there own, but what about the others? Do any of these need to be added to CSharpier?

AccessorDeclarationSyntax
AccessorListSyntax
AttributeArgumentListSyntax
AttributeArgumentSyntax
AttributeSyntax
AttributeTargetSpecifierSyntax
CatchDeclarationSyntax
CatchFilterClauseSyntax
ExplicitInterfaceSpecifierSyntax
FunctionPointerCallingConventionSyntax
FunctionPointerParameterListSyntax
FunctionPointerParameterSyntax
FunctionPointerUnmanagedCallingConventionListSyntax
FunctionPointerUnmanagedCallingConventionSyntax
InterpolationAlignmentClauseSyntax
InterpolationFormatClauseSyntax
JoinIntoClauseSyntax
OrderingSyntax
PositionalPatternClauseSyntax
PrimaryConstructorBaseTypeSyntax #39
PropertyPatternClauseSyntax
SubpatternSyntax
SwitchExpressionArmSyntax

I believe these are used internally in roslyn, but don't end up in the syntax tree.
BadDirectiveTriviaSyntax - happens when an unrecognized directive is encountered. Seems to be a compilation error so doesn't need formatting.
DefineDirectiveTriviaSyntax
DocumentationCommentTriviaSyntax
ElifDirectiveTriviaSyntax
ElseDirectiveTriviaSyntax
EndIfDirectiveTriviaSyntax
EndRegionDirectiveTriviaSyntax
ErrorDirectiveTriviaSyntax
IfDirectiveTriviaSyntax
LineDirectiveTriviaSyntax
LoadDirectiveTriviaSyntax
NullableDirectiveTriviaSyntax
PragmaChecksumDirectiveTriviaSyntax
PragmaWarningDirectiveTriviaSyntax
ReferenceDirectiveTriviaSyntax
RegionDirectiveTriviaSyntax
ShebangDirectiveTriviaSyntax
SkippedTokensTriviaSyntax
UndefDirectiveTriviaSyntax
WarningDirectiveTriviaSyntax

I believe all of these are used in xml doc
ConversionOperatorMemberCrefSyntax
CrefBracketedParameterListSyntax
CrefParameterListSyntax
CrefParameterSyntax
IndexerMemberCrefSyntax
NameMemberCrefSyntax
OperatorMemberCrefSyntax
QualifiedCrefSyntax
TypeCrefSyntax
XmlCDataSectionSyntax
XmlCommentSyntax
XmlCrefAttributeSyntax
XmlElementEndTagSyntax
XmlElementStartTagSyntax
XmlElementSyntax
XmlEmptyElementSyntax
XmlNameAttributeSyntax
XmlNameSyntax
XmlPrefixSyntax
XmlProcessingInstructionSyntax
XmlTextAttributeSyntax
XmlTextSyntax

We lose new lines in places we may want them, and insert them in places we probably shouldn't.
We also gain extra new lines at times.

We lose the new line after foo

#define foo

namespace Namespace

We add an extra new line before #pragma each time we format - #21

namespace Namespace
{
#pragma
    public class ClassName { }
}

There are some forms of comments we want inline, something like this breaks I think.

this.DoSomething(1, /* no break */ 2);

While trying to get my development environment going and all the tests running/passing, I SyntaxNodeJsonWriterGenerator tries to write to a file in a CSharpier.Parser directory:

which of course fails, because that directory doesn't exist.
I noticed that there's a version of SyntaxNodeJsonWriter.generated.cs in CSharpier directory, so I tried updating the path. However, the generated file seems to be drastically different than the one committed, and now generates a tonne of build errors.

Could you take a look at what's going on with that? I'm hoping it's a simple fix like a missing using in the generated file.

I believe CSharpier should follow the same rules for empty lines as prettier, which is

1. Collapse multiple blank lines into a single line
2. Empty lines at the start and end of blocks (and whole files) are removed. Files always end with a single new line
3. All other empty lines are preserved.

Review formatting of longer query expressions and determine how they should break.

Some examples

                var extension = GetExtensionForLanguageType(
                    (FrontEndResourceLanguage)Enum.Parse(
                        typeof(FrontEndResourceLanguage),
                        this.Language));

            this.Address1 = addressFields.FirstOrDefault(
                field => field.FieldName == "Address1");

// Invocation inside IfStatement
            if (customerOrder == null || !customerOrder.FulfillmentMethod.EqualsIgnoreCase(
                FulfillmentMethod.PickUp.ToString()))

This shows up when formatting https://github.com/dotnet/runtime
That repository has specific files that appear to be designed to fail on deep recursion.
For now there is an ugly hack in place to prevent getting too deep in the recursive print functions.
https://github.com/dotnet/roslyn/blob/master/src/Compilers/CSharp/Portable/Syntax/CSharpSyntaxWalker.cs has a potentially better way to guard against that.
Avoiding recursion would require a major rewrite, but may be more efficient.
This could provide some ideas for how to avoid recursion.
http://metacoding.azurewebsites.net/2016/08/16/how-to-avoid-recursion/

Ideally csharpier would be able to format razor files. It appears that razor files are parsed and converted to c#. See
https://github.com/davidebbo/RoslynRazorViewEngine/blob/master/RoslynRazorViewEngine/RoslynRazorViewEngine.cs#L127

How does prettier deal with html files, react files, etc?
Should the razor formatter be a prettier plugin instead? How would it parse/format the c# code in a razor file?

It should break like so

int x = 4,
    y = 5,
    z = 7;

While statements with BinaryExpressions(I think) don't format well.

while (directoryInfo.Name != "Automation" &&
directoryInfo.Parent != null)
{
    directoryInfo = directoryInfo.Parent;
}

We need to investigate which style cop rules need to be disabled to avoid conflicts with csharpier

public static Dictionary<Type,
    string[]> PropertiesByType = new Dictionary<Type, string[]>();
// should be
public static Dictionary<Type, string[]> PropertiesByType =
    new Dictionary<Type, string[]>();

There are probably also some edge cases around generics that aren't handled yet, because this feels like a weird way to break if it were too long by itself

public static Dictionary<Type,
    string[]>

When printing an if directive, a new line is being added each time.
Original Code

namespace Namespace
{
#pragma
    public class ClassName { }
}

Next Format

namespace Namespace
{

#pragma
    public class ClassName { }
}

The current method of printing results in deeply nested doc trees. See example below.

Could I rework (everything) so that this is simplified?
If I simplify things it will probably speed up performance, and it will make working on formatting easier.

Should print methods be passed a parent doc, which they append to?
Concat(Concat(Concat("get", "set"))) could be simplified to Concat("get", "set")
Concat(Concat("get"), Concat("set")) could be simplified to Concat("get", "set")
A potentially easier short term solution would be to have Concat examine what is being passed to it, and simplify it at that time.

Group(
    Concat(
        Concat(
            Line,
            Concat(
                "{"),
            Group(
                Concat(
                    Indent(
                        Concat(
                            Concat(
                                Line,
                                Concat(
                                    "get"),
                                Concat(
                                    ";")),
                            Concat(
                                Line,
                                Concat(
                                    "set"),
                                Concat(
                                    ";")))))),
            Line,
            Concat(
                "}")))),
null,
null))))),