xixiaofinland / afmt

Salesforce Apex code formatter written in Rust
20 stars 3 forks source link

SOQL WHERE IN clause losing list formatting #17

Closed aheber closed 2 days ago

aheber commented 5 days ago

On a SOQL WHERE IN clause, if I provide a list of options in parenthesis and separated by commas then the parenthesis and comma are lost on formatting. I also provided an example with a binding statement just to show it was working fine.

Example:

class TestClass {
  {
    List<SObject> records = [SELECT Id FROM Account WHERE Name IN ('test1', 'test2')];
    records = [SELECT Id FROM Account WHERE Name IN :records];
  }
}

Parse tree:

comparison_expression [2, 58] - [2, 83]
  field_identifier [2, 58] - [2, 62]
    identifier [2, 58] - [2, 62]
  set_comparison_operator [2, 63] - [2, 65]
  string_literal [2, 67] - [2, 74]
  string_literal [2, 75] - [2, 82]

Formatted:

class TestClass {
  {
    List<SObject> records = [SELECT Id FROM Account WHERE Name IN 'test1' 'test2'];
    records = [SELECT Id FROM Account WHERE Name IN :records];
  }
}
xixiaofinland commented 5 days ago

This is an interesting one.

For WHERE Name IN ('test1', 'test2'), it seems I miss a parent node of the two string_literal to determine where ( and ) sit, while WHERE Name IN :records has a bound_apex_expression node and it is anyway only one rather than two nodes.

I wonder how to handle the 1st scenario elegantly or shall we update the parser?

aheber commented 5 days ago

I vote update the parser. I probably should have grouped those to begin with. This is an easy thing for me to do and probably a difficult thing to guess at on your end unless you look for existing text nodes to hint at the correct behavior. I think it would be much easier if the parsed tree went from

Existing:

comparison_expression [0, 63] - [0, 86]
  field_identifier [0, 63] - [0, 65]
    identifier [0, 63] - [0, 65]
  set_comparison_operator [0, 66] - [0, 68]
  string_literal [0, 70] - [0, 77]
  string_literal [0, 78] - [0, 85]

to, when there are a pair of parenthesis

Proposed:

comparison_expression [0, 63] - [0, 86]
  field_identifier [0, 63] - [0, 65]
    identifier [0, 63] - [0, 65]
  set_comparison_operator [0, 66] - [0, 68]
  comparable_list <-- NEW container
    string_literal [0, 70] - [0, 77]
    string_literal [0, 78] - [0, 85]

Let me know if that works for you and I can get them added on my end.

xixiaofinland commented 4 days ago

absolutely parser master please go ahead :) @aheber

aheber commented 4 days ago

I've got the changes in tree-sitter-sfapex and I'll get a PR against this project with the fixes today or tomorrow.