From 0a952791b8f8c6487ac7154f376372c1be67c313 Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa <583144+Morriar@users.noreply.github.com> Date: Thu, 29 Jul 2021 11:52:26 -0400 Subject: [PATCH] Merge pull request #405 from Shopify/at-fix-rbi-sort Better sorting for RBI nodes --- lib/tapioca/rbi/rewriters/sort_nodes.rb | 30 +-- spec/tapioca/sort_nodes_spec.rb | 270 ++++++++++++++++++++++++ 2 files changed, 286 insertions(+), 14 deletions(-) create mode 100644 spec/tapioca/sort_nodes_spec.rb diff --git a/lib/tapioca/rbi/rewriters/sort_nodes.rb b/lib/tapioca/rbi/rewriters/sort_nodes.rb index 689af8c4e..88631f6b6 100644 --- a/lib/tapioca/rbi/rewriters/sort_nodes.rb +++ b/lib/tapioca/rbi/rewriters/sort_nodes.rb @@ -11,11 +11,11 @@ class SortNodes < Visitor def visit(node) return unless node.is_a?(Tree) visit_all(node.nodes) + original_order = node.nodes.map.with_index.to_h node.nodes.sort! do |a, b| - return 0 if a.is_a?(Mixin) || b.is_a?(Mixin) - res = node_rank(a) <=> node_rank(b) res = node_name(a) <=> node_name(b) if res == 0 + res = (original_order[a] || 0) <=> (original_order[b] || 0) if res == 0 res || 0 end end @@ -25,27 +25,29 @@ def visit(node) sig { params(node: Node).returns(Integer) } def node_rank(node) case node - when Group then kind_rank(node.kind) - when Include, Extend then 0 - when Helper then 1 - when TypeMember then 2 - when MixesInClassMethods then 3 - when TStructField then 4 - when TEnumBlock then 5 + when Group then group_rank(node.kind) + when Include, Extend then 10 + when Helper then 20 + when TypeMember then 30 + when MixesInClassMethods then 40 + when TStructField then 50 + when TEnumBlock then 60 when Method if node.name == "initialize" - 7 + 71 + elsif !node.is_singleton + 72 else - 8 + 73 end - when Scope, Const then 9 + when Scope, Const then 80 else - 10 + 100 end end sig { params(kind: Group::Kind).returns(Integer) } - def kind_rank(kind) + def group_rank(kind) case kind when Group::Kind::Mixins then 0 when Group::Kind::Helpers then 1 diff --git a/spec/tapioca/sort_nodes_spec.rb b/spec/tapioca/sort_nodes_spec.rb new file mode 100644 index 000000000..bc30ec824 --- /dev/null +++ b/spec/tapioca/sort_nodes_spec.rb @@ -0,0 +1,270 @@ +# typed: strict +# frozen_string_literal: true + +require "spec_helper" + +module Tapioca + module RBI + class SortNodesSpec < Minitest::HooksSpec + describe("can sort nodes") do + it("sorts constants") do + rbi = RBI::Tree.new + rbi << RBI::Const.new("C", "42") + rbi << RBI::Const.new("B", "42") + rbi << RBI::Const.new("A", "42") + + rbi.sort_nodes! + + assert_equal(<<~RBI, rbi.string) + A = 42 + B = 42 + C = 42 + RBI + end + + it("sorts modules") do + rbi = RBI::Tree.new + rbi << RBI::Module.new("C") + rbi << RBI::Module.new("B") + rbi << RBI::Module.new("A") + + rbi.sort_nodes! + + assert_equal(<<~RBI, rbi.string) + module A; end + module B; end + module C; end + RBI + end + + it("sorts classes") do + rbi = RBI::Tree.new + rbi << RBI::Class.new("C") + rbi << RBI::Class.new("B") + rbi << RBI::Class.new("A") + + rbi.sort_nodes! + + assert_equal(<<~RBI, rbi.string) + class A; end + class B; end + class C; end + RBI + end + + it("sorts constants and keeps original order in case of conflicts") do + rbi = RBI::Tree.new + rbi << RBI::Class.new("B") + rbi << RBI::Module.new("B") + rbi << RBI::Const.new("B", "42") + rbi << RBI::Const.new("A", "42") + rbi << RBI::Module.new("A") + rbi << RBI::Class.new("A") + + rbi.sort_nodes! + + assert_equal(<<~RBI, rbi.string) + A = 42 + module A; end + class A; end + class B; end + module B; end + B = 42 + RBI + end + + it("sorts methods") do + rbi = RBI::Tree.new + rbi << RBI::Method.new("m4") + rbi << RBI::Method.new("m3", is_singleton: true) + rbi << RBI::Method.new("m2", is_singleton: true) + rbi << RBI::Method.new("m1") + + rbi.sort_nodes! + + assert_equal(<<~RBI, rbi.string) + def m1; end + def m4; end + def self.m2; end + def self.m3; end + RBI + end + + it("does not sort mixins") do + rbi = RBI::Tree.new + rbi << RBI::MixesInClassMethods.new("E") + rbi << RBI::Extend.new("D") + rbi << RBI::Include.new("C") + rbi << RBI::Extend.new("B") + rbi << RBI::Include.new("A") + + rbi.sort_nodes! + + assert_equal(<<~RBI, rbi.string) + extend D + include C + extend B + include A + mixes_in_class_methods E + RBI + end + + it("sorts helpers") do + rbi = RBI::Tree.new + rbi << RBI::Helper.new("c") + rbi << RBI::Helper.new("b") + rbi << RBI::Helper.new("a") + + rbi.sort_nodes! + + assert_equal(<<~RBI, rbi.string) + a! + b! + c! + RBI + end + + it("sorts struct properties") do + rbi = RBI::Tree.new + rbi << RBI::TStructConst.new("d", "T") + rbi << RBI::TStructProp.new("c", "T") + rbi << RBI::TStructConst.new("b", "T") + rbi << RBI::TStructProp.new("a", "T") + + rbi.sort_nodes! + + assert_equal(<<~RBI, rbi.string) + prop :a, T + const :b, T + prop :c, T + const :d, T + RBI + end + + it("sorts structs") do + rbi = RBI::Tree.new + rbi << RBI::TStruct.new("D") + rbi << RBI::TStruct.new("C") + rbi << RBI::TStruct.new("B") + rbi << RBI::TStruct.new("A") + + rbi.sort_nodes! + + assert_equal(<<~RBI, rbi.string) + class A < ::T::Struct; end + class B < ::T::Struct; end + class C < ::T::Struct; end + class D < ::T::Struct; end + RBI + end + + it("sorts enums") do + rbi = RBI::Tree.new + rbi << RBI::TEnum.new("D") + rbi << RBI::TEnum.new("C") + rbi << RBI::TEnum.new("B") + rbi << RBI::TEnum.new("A") + + rbi.sort_nodes! + + assert_equal(<<~RBI, rbi.string) + class A < ::T::Enum; end + class B < ::T::Enum; end + class C < ::T::Enum; end + class D < ::T::Enum; end + RBI + end + + it("does nothing if all nodes are already sorted") do + rbi = RBI::Tree.new + rbi << RBI::Extend.new("M4") + rbi << RBI::Include.new("M3") + rbi << RBI::Extend.new("M2") + rbi << RBI::Include.new("M1") + rbi << RBI::Helper.new("h") + rbi << RBI::MixesInClassMethods.new("MICM") + rbi << RBI::TStructProp.new("SP1", "T") + rbi << RBI::TStructConst.new("SP2", "T") + rbi << RBI::TStructProp.new("SP3", "T") + rbi << RBI::TStructConst.new("SP4", "T") + rbi << RBI::Method.new("m1") + rbi << RBI::Method.new("m2") + rbi << RBI::Method.new("m3", is_singleton: true) + rbi << RBI::Const.new("A", "42") + rbi << RBI::Module.new("B") + rbi << RBI::TEnum.new("C") + rbi << RBI::TStruct.new("D") + rbi << RBI::Class.new("E") + + rbi.sort_nodes! + + assert_equal(<<~RBI, rbi.string) + extend M4 + include M3 + extend M2 + include M1 + h! + mixes_in_class_methods MICM + prop :SP1, T + const :SP2, T + prop :SP3, T + const :SP4, T + def m1; end + def m2; end + def self.m3; end + A = 42 + module B; end + class C < ::T::Enum; end + class D < ::T::Struct; end + class E; end + RBI + end + + it("sorts all nodes in tree") do + rbi = RBI::Tree.new + rbi << RBI::Const.new("A", "42") + rbi << RBI::Extend.new("M4") + rbi << RBI::Method.new("m1") + rbi << RBI::MixesInClassMethods.new("MICM") + rbi << RBI::Module.new("B") + rbi << RBI::Include.new("M3") + rbi << RBI::TStructConst.new("SP2", "T") + rbi << RBI::Method.new("m2") + rbi << RBI::TStruct.new("D") + rbi << RBI::TEnum.new("C") + rbi << RBI::Extend.new("M2") + rbi << RBI::TStructConst.new("SP4", "T") + rbi << RBI::Include.new("M1") + rbi << RBI::Helper.new("h") + rbi << RBI::Class.new("E") + rbi << RBI::TStructProp.new("SP1", "T") + rbi << RBI::Method.new("m3", is_singleton: true) + rbi << RBI::TStructProp.new("SP3", "T") + + rbi.sort_nodes! + + assert_equal(<<~RBI, rbi.string) + extend M4 + include M3 + extend M2 + include M1 + h! + mixes_in_class_methods MICM + prop :SP1, T + const :SP2, T + prop :SP3, T + const :SP4, T + def m1; end + def m2; end + def self.m3; end + A = 42 + module B; end + class C < ::T::Enum; end + class D < ::T::Struct; end + class E; end + RBI + end + end + end + end +end