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

SpaceBetweenMethods pass not keeping space around empty constructor with specific pass group #128

Closed
ferhado opened this issue Nov 15, 2023 · 3 comments

Comments

@ferhado
Copy link

ferhado commented Nov 15, 2023

I've encountered an issue with the SpaceBetweenMethods pass in a PHP environment, particularly when used in conjunction with the following group of passes: StripNewlineAfterClassOpen, StripNewlineAfterCurlyOpen, and StripNewlineWithinClassBody. The SpaceBetweenMethods pass does not maintain the expected space around an empty constructor.

Passes Used:

  • SpaceBetweenMethods
  • StripNewlineAfterClassOpen
  • StripNewlineAfterCurlyOpen
  • StripNewlineWithinClassBody

Current Behavior:

class ExampleClass {
  public function __construct(private SomeService $service) {}
  public function otherMethod() {
    // do something
  }
}

Expected Behavior:

class ExampleClass {

  public function __construct(private SomeService $service) {}

  public function otherMethod() {
    // do something
  }
}

The issue seems to arise when these specific passes are used together. I believe that the SpaceBetweenMethods pass should override the other passes to ensure the correct spacing is maintained, especially around empty constructors. This issue is critical for maintaining readability and consistency in the codebase.

Looking forward to any insights or fixes for this problem. Thank you!

@ferhado
Copy link
Author

ferhado commented Nov 15, 2023

I'd like to add a note to the issue: This issue also affects all empty methods, not just constructors, when using the same group of passes.

driade added a commit to driade/phpfmt8 that referenced this issue Nov 18, 2023
@driade
Copy link

driade commented Nov 18, 2023

Thanks, fixed, please confirm.

@ferhado
Copy link
Author

ferhado commented Nov 18, 2023

Confirmed, issue resolved with v1.1.29. Thank you

@ferhado ferhado closed this as completed Nov 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants