formatter: Add support for a few common functions and fix an LSP startup issue (#3190)

.vs/launch.vs.json

@@ -267,8 +267,8 @@
       "type" : "default",
       "project" : "CMakeLists.txt",
       "projectTarget" : "formatter.exe (bin\\formatter.exe)",
-      "name" : "Tools - Formatter",
-      "args" : ["--new", "--file", "C:\\Users\\xtvas\\Repos\\opengoal\\jak-project\\test-formatter.gc"]
+      "name" : "Tools - Formatter - Inplace",
+      "args" : ["--write", "--file", "C:\\Users\\xtvas\\Repos\\opengoal\\jak-project\\goal_src\\jak2\\pc\\util\\popup-menu-h.gc"]
     },
     {
       "type": "default",

common/formatter/formatter.cpp

@@ -114,6 +114,8 @@ void apply_formatting_config(
       if (predefined_config &&
           predefined_config->index_configs.find(i) != predefined_config->index_configs.end()) {
         apply_formatting_config(ref, predefined_config->index_configs.at(i));
+      } else if (predefined_config && predefined_config->default_index_config) {
+        apply_formatting_config(ref, predefined_config->default_index_config);
       } else {
         apply_formatting_config(ref);
       }
@@ -179,7 +181,7 @@ bool can_node_be_inlined(const FormatterTreeNode& curr_node, int cursor_pos) {
     return false;
   }
   // If this is set in the config, then the form is intended to be partially inlined
-  if (curr_node.formatting_config.inline_until_index != -1) {
+  if (curr_node.formatting_config.inline_until_index({})) {
     return false;
   }
   // let's see if we can inline the form all on one line to do that, we recursively explore
@@ -193,7 +195,8 @@ std::vector<std::string> apply_formatting(const FormatterTreeNode& curr_node,
                                           int cursor_pos = 0) {
   using namespace formatter_rules;
   if (!curr_node.token && curr_node.refs.empty()) {
-    return output;
+    // special case to handle an empty list
+    return {"()"};
   }
 
   // If its a token, just print the token and move on
@@ -211,6 +214,7 @@ std::vector<std::string> apply_formatting(const FormatterTreeNode& curr_node,
   //
   // This means we may combine elements onto the same line in this step.
   std::vector<std::string> form_lines = {};
+
   for (int i = 0; i < (int)curr_node.refs.size(); i++) {
     const auto& ref = curr_node.refs.at(i);
     // Add new line entry
@@ -265,10 +269,10 @@ std::vector<std::string> apply_formatting(const FormatterTreeNode& curr_node,
   }
 
   // Consolidate any lines if the configuration requires it
-  if (curr_node.formatting_config.inline_until_index != -1) {
+  if (curr_node.formatting_config.inline_until_index(form_lines)) {
     std::vector<std::string> new_form_lines = {};
     for (int i = 0; i < (int)form_lines.size(); i++) {
-      if (i < curr_node.formatting_config.inline_until_index) {
+      if (i < curr_node.formatting_config.inline_until_index(form_lines)) {
         if (new_form_lines.empty()) {
           new_form_lines.push_back(form_lines.at(i));
         } else {
@@ -311,7 +315,7 @@ std::vector<std::string> apply_formatting(const FormatterTreeNode& curr_node,
 }
 
 std::string join_formatted_lines(const std::vector<std::string> lines) {
-  // TODO - respect original file line endings?
+  // TODO - respect original file line endings
   return fmt::format("{}", fmt::join(lines, "\n"));
 }
 

common/formatter/rules/rule_config.cpp

@@ -5,13 +5,34 @@
 namespace formatter_rules {
 namespace config {
 
-// TODO - populate these more
-
 // TODO - this could be greatly simplified with C++20's designated initialization
 FormFormattingConfig new_flow_rule(int start_index) {
   FormFormattingConfig cfg;
   cfg.hang_forms = false;
-  cfg.inline_until_index = start_index;
+  cfg.inline_until_index = [start_index](const std::vector<std::string>& curr_lines) {
+    return start_index;
+  };
+  return cfg;
+}
+
+FormFormattingConfig new_flow_rule_prevent_inlining_indexes(
+    int start_index,
+    const std::vector<int>& inlining_preventation_indices) {
+  FormFormattingConfig cfg;
+  cfg.hang_forms = false;
+  cfg.inline_until_index = [start_index](std::vector<std::string> curr_lines) {
+    if (curr_lines.size() >= 4 && curr_lines.at(3) == "()") {
+      return 4;
+    }
+    return start_index;
+  };
+  for (const auto& index : inlining_preventation_indices) {
+    auto temp_config = std::make_shared<FormFormattingConfig>();
+    temp_config->prevent_inlining = true;
+    temp_config->hang_forms = false;
+    temp_config->indentation_width = 1;
+    cfg.index_configs.emplace(index, temp_config);
+  }
   return cfg;
 }
 
@@ -41,9 +62,27 @@ FormFormattingConfig new_binding_rule() {
   return cfg;
 }
 
+FormFormattingConfig new_pair_rule(bool combine_first_two_expr) {
+  FormFormattingConfig cfg;
+  cfg.hang_forms = false;
+  cfg.prevent_inlining = true;
+  cfg.combine_first_two_lines = combine_first_two_expr;
+  auto pair_config = std::make_shared<FormFormattingConfig>();
+  pair_config->hang_forms = false;
+  pair_config->indentation_width = 1;
+  cfg.default_index_config = pair_config;
+  return cfg;
+}
+
 const std::unordered_map<std::string, FormFormattingConfig> opengoal_form_config = {
+    {"case", new_pair_rule(true)},
+    {"cond", new_pair_rule(false)},
+    {"defmethod", new_flow_rule(3)},
+    {"deftype", new_flow_rule_prevent_inlining_indexes(3, {3, 4, 5})},
     {"defun", new_flow_rule(3)},
-    {"defmethod", new_flow_rule(4)},
-    {"let", new_binding_rule()}};
+    {"dotimes", new_flow_rule(2)},
+    {"let", new_binding_rule()},
+    {"when", new_flow_rule(2)},
+    {"with-dma-buffer-add-bucket", new_flow_rule(2)}};
 }  // namespace config
 }  // namespace formatter_rules

common/formatter/rules/rule_config.h

@@ -19,13 +19,16 @@ struct FormFormattingConfig {
   std::function<int(FormFormattingConfig, int)> indentation_width_for_index =
       [](FormFormattingConfig config, int /*index*/) { return config.indentation_width; };
   bool combine_first_two_lines =
-      false;  // NOTE - basically hang, but will probably stick around after hang is gone
-  int inline_until_index = -1;
+      false;  // NOTE - basically hang, but will probably stick around after hang is gone, may be
+              // redundant (inline_until_index!)
+  std::function<std::optional<int>(const std::vector<std::string>& curr_lines)> inline_until_index =
+      [](std::vector<std::string> curr_lines) { return std::nullopt; };
   bool has_constant_pairs = false;
-  bool prevent_inlining = false;
+  bool prevent_inlining = false;  // TODO - duplicate of below
   std::function<bool(FormFormattingConfig, int num_refs)> should_prevent_inlining =
       [](FormFormattingConfig config, int /*num_refs*/) { return config.prevent_inlining; };
   int parent_mutable_extra_indent = 0;
+  std::optional<std::shared_ptr<FormFormattingConfig>> default_index_config;
   std::unordered_map<int, std::shared_ptr<FormFormattingConfig>> index_configs = {};
 };
 

common/log/log.cpp

@@ -153,7 +153,9 @@ void set_file(const std::string& filename,
       }
     }
   } else {
-    complete_filename += ".log";
+    if (!str_util::ends_with(complete_filename, ".log")) {
+      complete_filename += ".log";
+    }
   }
 
   if (append) {

decompiler/config/jak2/ntsc_v1/art_info.jsonc

@@ -45,7 +45,7 @@
 
   // remap names for types in an entire file (higher priority)
   "file_override": {
-    "target-indax": {"target": "daxter-ag"} // in target-indax.gc, the remap for 'target' will be set to 'daxter-ag'
+    "target-indax": { "target": "daxter-ag" } // in target-indax.gc, the remap for 'target' will be set to 'daxter-ag'
   },
 
   // some art groups (like robotboss-ag) have a name for their model that differs

goal_src/jak2/pc/util/popup-menu-h.gc

@@ -10,7 +10,7 @@
   ((label string)
    (on-press (function none)))
   (:methods
-    (draw! (_type_ font-context dma-buffer) none)))
+   (draw! (_type_ font-context dma-buffer) none)))
 
 (deftype popup-menu-label (popup-menu-entry) ())
 
@@ -23,8 +23,8 @@
   ((entries (array popup-menu-entry))
    (curr-entry-index int32))
   (:methods
-    (draw! (_type_) none)
-    (move-up! (_type_) none)
-    (move-down! (_type_) none)
-    (press! (_type_) none)
-    (get-widest-label (_type_ font-context) float)))
+   (draw! (_type_) none)
+   (move-up! (_type_) none)
+   (move-down! (_type_) none)
+   (press! (_type_) none)
+   (get-widest-label (_type_ font-context) float)))

goal_src/jak2/pc/util/popup-menu.gc

@@ -17,32 +17,30 @@
     (set! (-> font-ctx origin y) 75.0)
     (with-dma-buffer-add-bucket ((buf (-> (current-frame) global-buf)) (bucket-id debug-no-zbuf1))
       ;; background border
-      (draw-sprite2d-xy buf 6 64
-        (+ 17 (the int (get-widest-label this font-ctx))) ;; width
-        (+ 17 (* 15 (-> this entries length))) ;; height
-        (new 'static 'rgba :r 255 :g 255 :b 255 :a 75))
+      (draw-sprite2d-xy buf
+                        6
+                        64
+                        (+ 17 (the int (get-widest-label this font-ctx))) ;; width
+                        (+ 17 (* 15 (-> this entries length))) ;; height
+                        (new 'static 'rgba :r 255 :g 255 :b 255 :a 75))
       ;; background
-      (draw-sprite2d-xy buf 7 65
-        (+ 15 (the int (get-widest-label this font-ctx))) ;; width
-        (+ 15 (* 15 (-> this entries length))) ;; height
-        (new 'static 'rgba :r 0 :g 0 :b 0 :a 255))
+      (draw-sprite2d-xy buf
+                        7
+                        65
+                        (+ 15 (the int (get-widest-label this font-ctx))) ;; width
+                        (+ 15 (* 15 (-> this entries length))) ;; height
+                        (new 'static 'rgba :r 0 :g 0 :b 0 :a 255))
       ;; menu contents
       (dotimes (i (-> this entries length))
         (cond
           ;; TODO - probably just handle this in the draw methods
-          ((type? (-> this entries i) popup-menu-label)
-           (set! (-> font-ctx color) (font-color progress-option-off)))
+          ((type? (-> this entries i) popup-menu-label) (set! (-> font-ctx color) (font-color progress-option-off)))
           ((type? (-> this entries i) popup-menu-flag)
            (set! (-> font-ctx color)
-                 (if (or ((-> (the-as popup-menu-flag (-> this entries i)) is-toggled?))
-                         (= i (-> this curr-entry-index)))
-                    (font-color cyan)
-                    (font-color default))))
-          (else
-           (set! (-> font-ctx color)
-                 (if (= i (-> this curr-entry-index))
-                    (font-color cyan)
-                    (font-color default)))))
+                 (if (or ((-> (the-as popup-menu-flag (-> this entries i)) is-toggled?)) (= i (-> this curr-entry-index)))
+                     (font-color cyan)
+                     (font-color default))))
+          (else (set! (-> font-ctx color) (if (= i (-> this curr-entry-index)) (font-color cyan) (font-color default)))))
         (draw! (-> this entries i) font-ctx buf)
         (set! (-> font-ctx origin y) (+ 15.0 (-> font-ctx origin y))))))
   (none))
@@ -57,13 +55,12 @@
 (defmethod move-down! ((this popup-menu))
   (set! (-> this curr-entry-index) (min (dec (-> this entries length)) (inc (-> this curr-entry-index))))
   ;; skip labels
-  (when (type?  (-> this entries (-> this curr-entry-index)) popup-menu-label)
+  (when (type? (-> this entries (-> this curr-entry-index)) popup-menu-label)
     (set! (-> this curr-entry-index) (min (dec (-> this entries length)) (inc (-> this curr-entry-index)))))
   (none))
 
 (defmethod press! ((this popup-menu))
-  (let ((entry (-> this entries (-> this curr-entry-index))))
-    ((-> entry on-press)))
+  (let ((entry (-> this entries (-> this curr-entry-index)))) ((-> entry on-press)))
   (none))
 
 (defmethod get-widest-label ((this popup-menu) (font-ctx font-context))

lsp/main.cpp

@@ -36,7 +36,8 @@
 
 void setup_logging(bool verbose, std::string log_file, bool disable_ansi_colors) {
   if (!log_file.empty()) {
-    lg::set_file(log_file, false, true, fs::path(log_file).parent_path().string());
+    lg::set_file(fs::path(log_file).filename().string(), false, true,
+                 fs::path(log_file).parent_path().string());
   }
   if (verbose) {
     lg::set_file_level(lg::level::debug);

test/common/formatter/corpus/cond-and-case.test.gc

@@ -0,0 +1,91 @@
+===
+Cond - Only Inlinable
+===
+
+(cond
+  ((type? (-> this entries i) popup-menu-label)
+   (set! x 1))
+  ((type? (-> this entries i) popup-menu-flag)
+   (set! x 2))
+  (else
+   (set! x 3)))
+
+---
+
+(cond
+  ((type? (-> this entries i) popup-menu-label) (set! x 1))
+  ((type? (-> this entries i) popup-menu-flag) (set! x 2))
+  (else (set! x 3)))
+
+===
+Cond - Mix and Match Inlining
+===
+
+(cond
+  ((type? (-> this entries i) popup-menu-flag)
+   (set! x (+ 89716239781623717236123 1239817239817298319287312 31298371298371239817231 231892739182739129837 231892739182739129837 231892739182739129837)))
+  ((type? (-> this entries i) popup-menu-label)
+   (set! x 1))
+  (else
+   (set! x 3)))
+
+---
+
+(cond
+  ((type? (-> this entries i) popup-menu-flag)
+   (set! x
+         (+ 89716239781623717236123
+            1239817239817298319287312
+            31298371298371239817231
+            231892739182739129837
+            231892739182739129837
+            231892739182739129837)))
+  ((type? (-> this entries i) popup-menu-label) (set! x 1))
+  (else (set! x 3)))
+
+===
+Case - Only Inlinable
+===
+
+(case (-> arg3 param 0)
+  (('dark)
+   (set! x 1))
+  (('light)
+   (set! x 2))
+  (else
+   (set! x 3)))
+
+---
+
+(case (-> arg3 param 0)
+  (('dark) (set! x 1))
+  (('light) (set! x 2))
+  (else (set! x 3)))
+
+===
+Case - Mix and Match Inlining
+===
+
+(case (-> arg3 param 0)
+  (('dark)
+   (set! x 1))
+  (('light)
+   (set! x (+ 89716239781623717236123 1239817239817298319287312 31298371298371239817231 231892739182739129837 231892739182739129837 231892739182739129837 231892739182739129837 231892739182739129837)))
+  (else
+   (set! x 3)))
+
+---
+
+(case (-> arg3 param 0)
+  (('dark) (set! x 1))
+  (('light)
+   (set! x
+         (+ 89716239781623717236123
+            1239817239817298319287312
+            31298371298371239817231
+            231892739182739129837
+            231892739182739129837
+            231892739182739129837
+            231892739182739129837
+            231892739182739129837)))
+  (else (set! x 3)))

test/common/formatter/corpus/iteration.test.gc

@@ -0,0 +1,15 @@
+===
+dotimes
+===
+
+(dotimes (i (-> this entries length))
+      (let ((label-len (-> (get-string-length (-> this entries i label) font-ctx) length)))
+        (when (> label-len max-len)
+          (set! max-len label-len))))
+
+---
+
+(dotimes (i (-> this entries length))
+  (let ((label-len (-> (get-string-length (-> this entries i label) font-ctx) length)))
+    (when (> label-len max-len)
+      (set! max-len label-len))))

test/common/formatter/corpus/lists.test.gc

@@ -22,3 +22,13 @@ Constant List - Too Long to Inline
  444444444444444444444444444
  555555555555555555555555555555555
  666666666666666666666666666666666666666)
+
+===
+Empty List
+===
+
+()
+
+---
+
+()

test/common/formatter/corpus/types.test.gc

@@ -0,0 +1,35 @@
+===
+Types - With Methods
+===
+
+(deftype popup-menu (basic)
+  ((entries (array popup-menu-entry))
+   (curr-entry-index int32))
+  (:methods
+    (draw! (_type_) none)
+    (move-up! (_type_) none)
+    (move-down! (_type_) none)
+    (press! (_type_) none)
+    (get-widest-label (_type_ font-context) float)))
+
+---
+
+(deftype popup-menu (basic)
+  ((entries (array popup-menu-entry))
+   (curr-entry-index int32))
+  (:methods
+   (draw! (_type_) none)
+   (move-up! (_type_) none)
+   (move-down! (_type_) none)
+   (press! (_type_) none)
+   (get-widest-label (_type_ font-context) float)))
+
+===
+Types - Empty
+===
+
+(deftype popup-menu-label (popup-menu-entry) ())
+
+---
+
+(deftype popup-menu-label (popup-menu-entry) ())

tools/formatter/main.cpp

@@ -47,13 +47,16 @@ int main(int argc, char** argv) {
   // TODO - support recursing directories
   // Read in source code
   const auto source_code = file_util::read_text_file(file_path);
-
   const auto result = formatter::format_code(source_code);
 
-  if (write_newfile && result) {
+  if (result) {
     // TODO - i don't like this implementation, return a new string instead
-    if (str_util::replace(file_path, ".gc", ".new.gc")) {
+    if (write_inplace) {
       file_util::write_text_file(file_path, result.value());
+    } else if (write_newfile) {
+      if (str_util::replace(file_path, ".gc", ".new.gc")) {
+        file_util::write_text_file(file_path, result.value());
+      }
     }
   }