xixiaofinland / afmt

Salesforce Apex code formatter written in Rust
MIT License
23 stars 3 forks source link

Comment in map construction do really weird things #12

Open aheber opened 1 month ago

aheber commented 1 month ago

This one is extra interesting, if I litter comments into map initialization syntax then afmt's printer simply joins the elements together. Personally I'd blame the parsing library for not grouping these together as paired key => value constructs. If you agree then we can send him a strongly worded letter asking that he change that. Because all elements appears in one big array then you end up grouping comments in as keys or values in the order they appear instead of as they used to be.

I do think this is a parser improvement first which will probably break a few things if I nest these down another level. I'll wait for your input before I make any changes.

Example:

class TestClass {
  Map<Integer, String> Test_Map = new Map<Integer, String>{
    1 => 'val1',
    2 => 'val2',
    // test comment 1
    3 => 'val3'
    // test comment 2
  };
}

Formatted: (line breaks added for clarity, afmt merged them onto a single line)

class TestClass {
  Map<Integer, String> Test_Map = new Map<Integer, String>{ 
    1 => 'val1',
    2 => 'val2',
    // test comment 1 => 3,
    'val3' =>   // test comment 2
  };
}
aheber commented 1 month ago

I think I'd recommend I change the parse tree to group these together better. I think visually it makes more sense.

new Map<String, String>{'foo'=>'bar', 'fizz'=>'buzz'};

Existing:

value: map_initializer [0, 23] - [0, 53]
  string_literal [0, 24] - [0, 29]
  string_literal [0, 31] - [0, 36]
  string_literal [0, 38] - [0, 44]
  string_literal [0, 46] - [0, 52]

Proposed:

value: map_initializer [0, 23] - [0, 53]
  map_key_initializer <- NEW container
    string_literal [0, 24] - [0, 29]
    string_literal [0, 31] - [0, 36]
  map_key_initializer <- NEW container
    string_literal [0, 38] - [0, 44]
    string_literal [0, 46] - [0, 52]

Let me know what you think.

xixiaofinland commented 1 month ago

the proposed one definitely is more organized, I'm more than happy to use it :) @aheber