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

Inexplicable behavior in "Align parameters and components" #53

Closed
ConjuringCoffee opened this issue Jun 7, 2023 · 5 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@ConjuringCoffee
Copy link
Contributor

ConjuringCoffee commented Jun 7, 2023

I found an example with the rule "Align parameters and components" that doesn't make sense to me. Can someone please explain this behavior to me?

CLASS lcl_example DEFINITION CREATE PUBLIC.

  PUBLIC SECTION.

  PROTECTED SECTION.

  PRIVATE SECTION.
    DATA:
      BEGIN OF variable,
        short_1 TYPE bapiret2,
        short_2 TYPE bapiret2,
        short_3 TYPE bapiret2,
        short_4 TYPE bapiret2,
        short_5 TYPE bapiret2,
      END OF variable.

    METHODS example.
ENDCLASS.


CLASS lcl_example IMPLEMENTATION.
  METHOD example.
    variable = VALUE #( short_1 = VALUE #(
        number     = '001'
        message_v1 = '1'
        message    = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1' )
                        short_2 = VALUE #(
                            number     = '002'
                            message_v1 = '2'
                            message    = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa2' )
                        short_3 = VALUE #(
                            number     = '003'
                            message_v1 = '3'
                            message    = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa3' )
                        short_4 = VALUE #(
                            number     = '004'
                            message_v4 = '4'
                            message    = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa4' )
                        short_5 = VALUE #(
                            number     = '005'
                            message_v4 = '5'
                            message    = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa5' ) ).
  ENDMETHOD.
ENDCLASS.

I would have expected this or a similar result. The content in message has the same length every time, so that can't be the reason....

  METHOD example.
    variable = VALUE #( short_1 = VALUE #(
                            number     = '001'
                            message_v1 = '1'
                            message    = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1' )
                        short_2 = VALUE #(
                            number     = '002'
                            message_v1 = '2'
                            message    = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa2' )
                        short_3 = VALUE #(
                            number     = '003'
                            message_v1 = '3'
                            message    = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa3' )
                        short_4 = VALUE #(
                            number     = '004'
                            message_v4 = '4'
                            message    = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa4' )
                        short_5 = VALUE #(
                            number     = '005'
                            message_v4 = '5'
                            message    = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa5' ) ).
  ENDMETHOD.

Here are my options:
image

@jmgrassau jmgrassau added the bug Something isn't working label Jun 7, 2023
@jmgrassau
Copy link
Member

Hi ConjuringCoffee,

good point…

The reason for this behavior is that with your last option ("Allow line starts left of assignment operator: only to keep maximum line length"), ABAP cleaner is allowed to move line starts left of the = in "emergency" situations, when maximum line length would be exceeded with normal indent. In such a case, ABAP cleaner uses <emergency indent> = <indent of previous line> + 4. That's why the '001' VALUE expression ends up in a different position (indent of "variable =" + 4) than the other expressions (indent of "short_2" + 4).

Of course it would be more consistent to only use <indent of "short_1"> + 4 in the first case – on the other hand, this may not help much in other examples where the inner VALUE already begins very far to the right.

But you are right, of course, this looks very odd. Will have to experiment a bit to find a good solution to this!

Kind regards,
Jörg-Michael

@jmgrassau
Copy link
Member

Hi ConjuringCoffee,

this was a rather easy fix (I hope), so "Align parameters and components" now makes sure that the components in the inner VALUE constructors are NOT moved further to the left than the components of the outer VALUE constructor. The fixed rule will also automatically "repair" the messed up example above.

If you reduce "Maximum line length A (normal)" step by step (press PageUp / PageDown for steps of 10), you can nicely watch the cleanup rule trying to observe line length:

maximum = 110:

    rs_result = VALUE #( comp_1 = VALUE #( num  = '001'
                                           text = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1' )
                         comp_2 = VALUE #( num  = '002'
                                           text = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa2' ) ).

maximum = 100:

    rs_result = VALUE #( comp_1 = VALUE #(
                             num  = '001'
                             text = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1' )
                         comp_2 = VALUE #(
                             num  = '002'
                             text = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa2' ) ).

maximum = 90:

    rs_result = VALUE #(
        comp_1 = VALUE #( num  = '001'
                          text = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1' )
        comp_2 = VALUE #( num  = '002'
                          text = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa2' ) ).

maximum = 80:

    rs_result = VALUE #(
        comp_1 = VALUE #(
            num  = '001'
            text = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1' )
        comp_2 = VALUE #(
            num  = '002'
            text = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa2' ) ).

Kind regards,
Jörg-Michael

P.S.: A non-ideal (but nevertheless correct) result is e.g. produced at maximum 86, where the additional ")." of the second inner VALUE constructor makes the difference:

    rs_result = VALUE #(
        comp_1 = VALUE #( num  = '001'
                          text = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1' )
        comp_2 = VALUE #(
            num  = '002'
            text = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa2' ) ).

@ConjuringCoffee
Copy link
Contributor Author

ConjuringCoffee commented Jun 12, 2023

Great, thanks! Would it be possible to avoid your last non-ideal example as well? I'm sure it will come up some time in the future otherwise 😀

I can think of two possibilities right now:

  1. Create an option to define trailing brackets and dots as an exception to the length rule
  2. Create an option in the other rule to put the brackets on the next line
  3. After calculating the result for comp_2, go back and adjust the other results so it'd look like this:
    rs_result = VALUE #(
        comp_1 = VALUE #( 
            num  = '001'
            text = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1' )
        comp_2 = VALUE #(
            num  = '002'
            text = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa2' ) ).

Personally, I'd prefer the third approach, but of course I don't know if that is even feasible with your current approach.

EDIT: I think I might have a case that fits this topic even after the latest update. I'll reduce this to a minimal working example...

@ConjuringCoffee
Copy link
Contributor Author

I have opened the new issue #56 for this topic because the original problem I reported is solved now. It can be closed from my point of view.

@jmgrassau
Copy link
Member

Hi ConjuringCoffee,

excellent, thanks a lot! And thanks so much for taking your time to report, comment, and even retest all these issues!

Kind regards,
Jörg-Michael

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants