Wrong formatting for type spec and body

[jgebal]

First of, thank you Philip for tremendous work and effort you took to understand the formatter and implement custom formatting settings.

I've downloaded latest version of settings and tried to apply formatter on some of utPLSQL sources.

I think utPLSQL can be a good battlefield for formatter as we use quite a lot of PL/SQL and SQL language features.

The only setting I have changed was to keep keywords as lowercase.

When formatting type spec, the methods are not aligned properly.
Some examples below:

create or replace type ut_realtime_reporter force under ut_output_reporter_base (
  /*
  utPLSQL - Version 3
  Copyright 2016 - 2019 utPLSQL Project

  Licensed under the Apache License, Version 2.0 (the "License"):
  you may not use this file except in compliance with the License.
  You may obtain a copy of the License at

      http://www.apache.org/licenses/LICENSE-2.0

  Unless required by applicable law or agreed to in writing, software
  distributed under the License is distributed on an "AS IS" BASIS,
  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  See the License for the specific language governing permissions and
  limitations under the License.
  */
  
  /**
   * Cached XML header to be used for every XML document
   */
   xml_header             varchar2(4000),

  /**
   * Total number of all tests in the run (incl. disabled tests).
   */
   total_number_of_tests  integer, 
 
  /**
   * Currently executed test number.
   */
   current_test_number    integer,

  /**
   * Current indentation in logical tabs.
   */
   current_indent         integer,
  
  /**
   * Buffers lines to be printed.
   */
   print_buffer           ut_varchar2_rows,
  
  /**
   * The realtime reporter.
   * Provides test results in a XML format, for clients such as SQL Developer interested in showing progressing details.
   */ constructor function ut_realtime_reporter (
      self in out nocopy ut_realtime_reporter
   ) return self as result,

  /**
   * Provides meta data of complete run in advance.
   */ overriding member procedure before_calling_run (
      self   in out nocopy ut_realtime_reporter,
      a_run  in ut_run
   ),

  /**
   * Provides meta data of a completed run.
   */ overriding member procedure after_calling_run (
      self   in out nocopy ut_realtime_reporter,
      a_run  in ut_run
   ),

  /**
   * Indicates the start of a test suite execution.
   */ overriding member procedure before_calling_suite (
      self     in out nocopy ut_realtime_reporter,
      a_suite  in ut_logical_suite
   ),

  /**
   * Provides meta data of completed test suite.
   */ overriding member procedure after_calling_suite (
      self     in out nocopy ut_realtime_reporter,
      a_suite  in ut_logical_suite
   ),

  /**
   * Indicates the start of a test.
   */ overriding member procedure before_calling_test (
      self    in out nocopy ut_realtime_reporter,
      a_test  in ut_test
   ),

  /**
   * Provides meta data of a completed test. 
   */ overriding member procedure after_calling_test (
      self    in out nocopy ut_realtime_reporter,
      a_test  in ut_test
   ),

  /**
   * Provides the description of this reporter.
   */ overriding member function get_description return varchar2,

  /**
   * Prints the start tag of a XML node with an optional attribute.
   */ member procedure print_start_node (
      self          in out nocopy ut_realtime_reporter,
      a_node_name   in  varchar2,
      a_attr_name   in  varchar2 default null,
      a_attr_value  in  varchar2 default null
   ),
  
  /**
   * Prints the end tag of a XML node.
   */ member procedure print_end_node (
      self    in out nocopy ut_realtime_reporter,
      a_name  in varchar2
   ),

  /**
   * Prints a child node with content. Special characters are encoded.
   */ member procedure print_node (
      self       in out nocopy ut_realtime_reporter,
      a_name     in  varchar2,
      a_content  in  clob
   ),
  
  /**
   * Prints a child node with content. Content is passed 1:1 using CDATA.
   */ member procedure print_cdata_node (
      self       in out nocopy ut_realtime_reporter,
      a_name     in  varchar2,
      a_content  in  clob
   ),

  /**
   * Prints a line of the resulting XML document using the current indentation.
   * a_indent_summand_before is added before printing a line.
   * a_indent_summand_after is added after printing a line.
   * All output is produced through this function.
   */ member procedure print_xml_fragment (
      self                     in out nocopy ut_realtime_reporter,
      a_fragment               in  clob,
      a_indent_summand_before  in  integer default 0,
      a_indent_summand_after   in  integer default 0
   ),
  
  /**
   * Flushes the local print buffer to the output buffer.
   */ member procedure flush_print_buffer (
      self         in out nocopy ut_realtime_reporter,
      a_item_type  in varchar2
   )
) not final
/

In above, the member procedure is not placed on new line.

It is even more visible here:

create or replace type ut_xunit_reporter under ut_junit_reporter (
  /*
  utPLSQL - Version 3
  Copyright 2016 - 2019 utPLSQL Project

  Licensed under the Apache License, Version 2.0 (the "License"):
  you may not use this file except in compliance with the License.
  You may obtain a copy of the License at

      http://www.apache.org/licenses/LICENSE-2.0

  Unless required by applicable law or agreed to in writing, software
  distributed under the License is distributed on an "AS IS" BASIS,
  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  See the License for the specific language governing permissions and
  limitations under the License.
  */
  /**
   * The XUnit reporter.
   * Provides outcomes in a format conforming with JUnit4 as defined in:
   *  https://gist.github.com/kuzuha/232902acab1344d6b578
   */
   constructor function ut_xunit_reporter (
      self in out nocopy ut_xunit_reporter
   ) return self as result, overriding member function get_description return varchar2
) not final
/

When formatting type body the behavior is also odd with some keywords:

create or replace type body ut_xunit_reporter is
  /*
  utPLSQL - Version 3
  Copyright 2016 - 2019 utPLSQL Project

  Licensed under the Apache License, Version 2.0 (the "License"):
  you may not use this file except in compliance with the License.
  You may obtain a copy of the License at

      http://www.apache.org/licenses/LICENSE-2.0

  Unless required by applicable law or agreed to in writing, software
  distributed under the License is distributed on an "AS IS" BASIS,
  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  See the License for the specific language governing permissions and
  limitations under the License.
  */
   constructor function ut_xunit_reporter (
      self in out nocopy ut_xunit_reporter
   ) return self as result is
   begin
      self.init($$plsql_unit);
      return;
   end;
overriding
   member function get_description return varchar2 as
   begin
      return 'Depracated reporter. Please use Junit.
    Provides outcomes in a format conforming with JUnit 4 and above as defined in: https://gist.github.com/kuzuha/232902acab1344d6b578';
   end;
end;
/

All was done running SQLDeveloper 19.4 (JDK 8 included) and Windows 10 Pro x64.

[PhilippSalvisberg]

I try to simplify the example.

1. Apply the settings according this repository
2. Change advanced settings Keyword case to lower.
3. Then the following code should be formatted as shown below.

Object Type Specification

create or replace type ut_xunit_reporter under ut_junit_reporter (
   constructor function ut_xunit_reporter (
      self in out nocopy ut_xunit_reporter
   ) return self as result,
   overriding member function get_description return varchar2
) not final
/

Object Type Body

create or replace type body ut_xunit_reporter is
   constructor function ut_xunit_reporter (
      self in out nocopy ut_xunit_reporter
   ) return self as result is
   begin
      self.init($$plsql_unit);
      return;
   end;
   overriding member function get_description return varchar2 as
   begin
      return 'Depracated reporter. Please use Junit.
    Provides outcomes in a format conforming with JUnit 4 and above as defined in: https://gist.github.com/kuzuha/232902acab1344d6b578';
   end;
end;
/

Is this correct?

[jgebal]

That is correct.

It seems like member procedures functions are not prefixed with newline.

I'm not sure if parsing tree is distinguishing between:

• overriding
• final
• not final
• static

final/not final and member can be combined.

I noticed that IntelliJ is formatting those in an interesting way (more Java-like), so that the static, final, member, overrriding keywords are in separate lines (like java annotations).

Example:

create or replace type ut_realtime_reporter force under ut_output_reporter_base(
  /**
   * Provides meta data of complete run in advance.
   */
  overriding
  member
  procedure before_calling_run(
    self in out nocopy ut_realtime_reporter,
    a_run in ut_run
    )
)
  not final
/

At first I really didn't like it, but after a moment I understood the concept and it is an interesting approach.

Not sure if that would be your (or mine) preference though.

Your suggested fix is definitely valid.

[PhilippSalvisberg]

Thanks @jgebal . Yes we have to to ensure that a line break is added in all the cases you've mentioned. This should not be a problem when using the parent nodes, method_specification (spec) and body_adt_field (body).

My default Java formatter does not add line breaks between method modifiers, so I'm not used to this formatting variant. So maybe that's why I do not like it and therefore will not add it.

[PhilippSalvisberg]

Thank you, Jacek for reporting this. As a side-effect of this change, the empty lines before the comments are preserved 😃.