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

Seems like incorrect break for chained property and method calls #956

Closed
kazrac opened this issue Sep 8, 2023 · 1 comment · Fixed by #967
Closed

Seems like incorrect break for chained property and method calls #956

kazrac opened this issue Sep 8, 2023 · 1 comment · Fixed by #967
Milestone

Comments

@kazrac
Copy link

kazrac commented Sep 8, 2023

In https://playground.csharpier.com/ I have played around with chains and following does not seem right:
Input:

someLongNameField
    .Method0()
    .Property0
    .Property1
    .Property2
    .Method1()
    .Property3
    .Array1[1]
    .Property4
    .Property5
    .Method2()
    .Method3("some input")
    .Method2()
    .Property6
    .Property7
    .Array2[2]
    .Property8
    .Method4("some input")
    .Property9
    .Property10;

output:

someLongNameField.Method0().Property0.Property1.Property2.Method1().Property3.Array1[
    1
].Property4.Property5
    .Method2()
    .Method3("some input")
    .Method2()
    .Property6.Property7.Array2[2].Property8
    .Method4("some input")
    .Property9.Property10;

property and method chains are broken differently.

Other example:
input

			return _someResolver
				.Resolve(
					(
						raw
							.Settings
							.AttributeTemplate
							.Settings
							.OutputSettings[raw.TestFramework]
							.AttributeTemplate
							.Settings
							.AttributeTemplate
							.Settings
							.OutputSettings[raw.TestFramework]
							.AttributeTemplate,
						new
						{
							ClassName = className,
							TestClassName = raw.MethodSymbol.ContainingType.Name,
							TestName = raw.MethodSymbol.Name
						})
				);

output:

return _someResolver.Resolve(
    (
        raw.Settings.AttributeTemplate.Settings.OutputSettings[raw.TestFramework]
            .AttributeTemplate
            .Settings
            .AttributeTemplate
            .Settings
            .OutputSettings[raw.TestFramework].AttributeTemplate,
        new
        {
            ClassName = className,
            TestClassName = raw.MethodSymbol.ContainingType.Name,
            TestName = raw.MethodSymbol.Name
        }
    )
);
@belav
Copy link
Owner

belav commented Sep 8, 2023

The breaking of long chains uses the same logic as prettier, although it looks like there is a bug with the 1st case. Prettier does this with it. There is definitely a bug with the current way csharpier breaks it.

someLongNameField
  .Method0()
  .Property0.Property1.Property2.Method1()
  .Property3.Array1[1].Property4.Property5.Method2()
  .Method3("some input")
  .Method2()
  .Property6.Property7.Array2[2].Property8.Method4("some input").Property9
  .Property10;

The 2nd case ends up like this in prettier

_someResolver.Resolve(
  (raw.Settings.AttributeTemplate.Settings.OutputSettings[raw.TestFramework]
    .AttributeTemplate.Settings.AttributeTemplate.Settings.OutputSettings[
    raw.TestFramework
  ].AttributeTemplate,
  {
    ClassName: className,
    TestClassName: raw.MethodSymbol.ContainingType.Name,
    TestName: raw.MethodSymbol.Name,
  }),
);

If I remember correctly, the logic tries to group property accessors onto a single line if they fit. It seems worth exploring breaking them instead. At the very least fixing the first case, which seems to be a bug.

#451 may be related.

@belav belav added this to the 0.26.0 milestone Sep 8, 2023
shocklateboy92 added a commit that referenced this issue Oct 10, 2023
…ains. (#967)

* notes

* Getting element access to break more consistently with invocations.

closes #956

* Getting properties breaking consistent with invocations

* Fixing more edge cases

* some self review

---------

Co-authored-by: Lasath Fernando <devel@lasath.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants