Skip to content

Commit df1d318

Browse files
shagemanshageman
authored
Add info on node todos (#57)
* Rename existing todos configs to “relationship” configs * Add package-based todos * Update test cassettes * Bump gem version
1 parent 83c0029 commit df1d318

30 files changed

Lines changed: 375 additions & 77 deletions

‎Gemfile.lock‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
PATH
22
remote: .
33
specs:
4-
visualize_packs (0.5.20)
4+
visualize_packs (0.5.21)
55
packs-specification
66
parse_packwerk (>= 0.20.0)
77
sorbet-runtime

‎bin/visualize_packs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,12 @@ OptionParser.new do |opt|
1717
opt.on('--no-layers', "Don't show architectural layers") { |o| options.show_layers = false }
1818
opt.on('--no-visibility-arrows', "Don't show visibility arrows") { |o| options.show_visibility = false }
1919

20-
opt.on('--no-todo-arrows', "Don't show pack todos") { |o| options.show_todos = false }
21-
opt.on("--only-todo-types=STRING", "Show only the selected types of todos. Comma-separated list of #{EdgeTodoTypes.values.map &:serialize}") { |o| options.only_todo_types = o.to_s.split(",").uniq.map { EdgeTodoTypes.deserialize(_1) } }
22-
opt.on("--use-todos-for-layout", "Show only the selected types of todos. Comma-separated list of #{EdgeTodoTypes.values.map &:serialize}") { |o| options.use_todos_for_layout = true }
20+
opt.on('--no-todo-edges', "Don't show todos for package relationships") { |o| options.show_relationship_todos = false }
21+
opt.on("--edge-todo-types=STRING", "Show only the selected types of relationship todos. Comma-separated list of #{EdgeTodoTypes.values.map &:serialize}") { |o| options.relationship_todo_types = o.to_s.split(",").uniq.map { EdgeTodoTypes.deserialize(_1) } }
22+
opt.on("--use-edge-todos-for-layout", "Show only the selected types of relationship todos. Comma-separated list of #{EdgeTodoTypes.values.map &:serialize}") { |o| options.use_relationship_todos_for_layout = true }
2323

2424
opt.on('--no-teams', "Don't show team colors") { |o| options.show_teams = false }
25+
opt.on('--no-node-todos', "Don't show package-based todos") { |o| options.show_node_todos = false }
2526

2627
opt.on('--focus-pack=STRING', "Focus on a specific pack(s). Comma-separated list of packs. Wildcards supported: 'packs/*'") { |o| options.focus_pack = o.to_s.split(",") }
2728
opt.on('--focus-pack-edge-mode=STRING', "If focus-pack is set, this shows only between focussed packs (when set to none) or the edges into / out of / in and out of the focus packs to non-focus packs (which will be re-added to the graph). One of #{FocusPackEdgeDirection.values.map &:serialize}") { |o| options.show_only_edges_to_focus_pack = FocusPackEdgeDirection.deserialize(o) }

‎diagram_examples.png‎

58.2 KB
Loading

‎lib/graph.dot.erb‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,18 @@ digraph package_diagram {
4646
<table border='0' cellborder='1' cellspacing='0' cellpadding='4'>
4747
<tr> <td port='private'> <%= package.name -%> </td> </tr>
4848
</table>
49-
</td></tr></table>
49+
</td></tr>
50+
<%- if options.show_node_todos && node_protection.(package.name) != "" -%>
51+
<tr><td CELLPADDING='1' ALIGN='RIGHT'><%= node_protection.(package.name) %></td></tr>
52+
<%- end -%>
53+
</table>
5054
>
5155
<%- else -%><
5256
<table border='0' cellborder='1' cellspacing='0' cellpadding='4'>
5357
<tr> <td align='left'> <%= package.name -%> </td> </tr>
58+
<%- if options.show_node_todos && node_protection.(package.name) != "" -%>
59+
<tr><td CELLPADDING='1' ALIGN='RIGHT'><%= node_protection.(package.name) %></td></tr>
60+
<%- end -%>
5461
</table>
5562
>
5663
<%- end -%>
@@ -79,16 +86,16 @@ digraph package_diagram {
7986
<%- end -%>
8087
<%- end -%>
8188
<%- end -%>
82-
<%- if options.show_todos -%>
89+
<%- if options.show_relationship_todos -%>
8390
<%- all_packages.each do |package| -%>
84-
<%- filtered_todos = package.violations.select { options.only_todo_types.include?(EdgeTodoTypes.deserialize(_1.type)) } -%>
91+
<%- filtered_todos = package.violations.select { options.relationship_todo_types.include?(EdgeTodoTypes.deserialize(_1.type)) } -%>
8592
<%- todos_by_package = filtered_todos.group_by(&:to_package_name) -%>
8693
<%- todos_by_package.keys.each do |todos_to_package| -%>
8794
<%- todo_types = todos_by_package[todos_to_package].group_by(&:type) -%>
8895
<%- todo_types.keys.each do |todo_type| -%>
8996
<%- if show_edge.call(package.name, todos_to_package) -%>
9097
"<%= package.name -%>" -> "<%= todos_to_package -%>"<%= todo_type == 'privacy' ? ':private' : '' -%> [
91-
<%- if !options.use_todos_for_layout -%>
98+
<%- if !options.use_relationship_todos_for_layout -%>
9299
constraint=false
93100
<%- end -%>
94101
# headlabel="<%= todo_type -%>"
@@ -143,23 +150,23 @@ digraph package_diagram {
143150
N [ fontsize=12 shape=box label="package"]
144151
M -> N [label="visibile to" <%= VisualizePacks::ArrowHead::ConfiguredVisibileTo.serialize %>]
145152
<%- end -%>
146-
<%- if options.show_todos -%>
147-
<%- if options.only_todo_types.include?(EdgeTodoTypes::Privacy) -%>
153+
<%- if options.show_relationship_todos -%>
154+
<%- if options.relationship_todo_types.include?(EdgeTodoTypes::Privacy) -%>
148155
C [ fontsize=12 shape=box label="package"]
149156
D [ fontsize=12 shape=box label="package"]
150157
C -> D [label="privacy todo" <%= VisualizePacks::ArrowHead::PrivacyTodo.serialize %>]
151158
<%- end -%>
152-
<%- if options.only_todo_types.include?(EdgeTodoTypes::Architecture) -%>
159+
<%- if options.relationship_todo_types.include?(EdgeTodoTypes::Architecture) -%>
153160
E [ fontsize=12 shape=box label="package"]
154161
F [ fontsize=12 shape=box label="package"]
155162
E -> F [label="architecture todo" <%= VisualizePacks::ArrowHead::ArchitectureTodo.serialize %>]
156163
<%- end -%>
157-
<%- if options.only_todo_types.include?(EdgeTodoTypes::Visibility) -%>
164+
<%- if options.relationship_todo_types.include?(EdgeTodoTypes::Visibility) -%>
158165
G [ fontsize=12 shape=box label="package"]
159166
H [ fontsize=12 shape=box label="package"]
160167
G -> H [label="visibility todo" <%= VisualizePacks::ArrowHead::VisibilityTodo.serialize %>]
161168
<%- end -%>
162-
<%- if options.only_todo_types.include?(EdgeTodoTypes::Dependency) -%>
169+
<%- if options.relationship_todo_types.include?(EdgeTodoTypes::Dependency) -%>
163170
I [ fontsize=12 shape=box label="package"]
164171
J [ fontsize=12 shape=box label="package"]
165172
I -> J [label="dependency todo" <%= VisualizePacks::ArrowHead::DependencyTodo.serialize %>]

‎lib/visualize_packs.rb‎

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ def self.package_graph!(options, raw_config, packages)
3131

3232
show_edge = show_edge_builder(options, all_package_names)
3333
node_color = node_color_builder()
34+
node_protection = package_based_todos_text_maker()
3435
max_todo_count = max_todo_count(all_packages, show_edge, options)
3536

3637
title = diagram_title(options, max_todo_count)
@@ -80,22 +81,23 @@ def self.diagram_title(options, max_todo_count)
8081
options.show_legend ? nil : "legend",
8182
options.show_layers ? nil : "layers",
8283
options.show_dependencies ? nil : "dependencies",
83-
options.show_todos ? nil : "todos",
84+
options.show_relationship_todos ? nil : "edge todos",
8485
options.show_privacy ? nil : "privacy",
8586
options.show_teams ? nil : "teams",
87+
options.show_node_todos ? nil : "node todos",
8688
options.show_visibility ? nil : "visibility",
8789
options.roll_nested_into_parent_packs ? "nested packs" : nil,
8890
options.show_nested_relationships ? nil : "nested relationships",
8991
].compact.join(', ').strip
9092
hidden_aspects_title = hidden_aspects != '' ? "Hiding #{hidden_aspects}" : nil
9193

92-
todo_types = EdgeTodoTypes.values.size == options.only_todo_types.size ? nil : "Only #{options.only_todo_types.map &:serialize} todos",
94+
todo_types = EdgeTodoTypes.values.size == options.relationship_todo_types.size ? nil : "Only #{options.relationship_todo_types.map &:serialize} todos",
9395

9496
exclusions = options.exclude_packs.empty? ? nil : "Excluding pack#{options.exclude_packs.size > 1 ? 's' : ''}: #{limited_sentence(options.exclude_packs)}",
9597

9698
main_title = [focus_info, hidden_aspects_title, todo_types, exclusions].compact.join('. ')
9799

98-
if options.show_todos && max_todo_count
100+
if options.show_relationship_todos && max_todo_count
99101
sub_title = "<br/><font point-size='12'>Widest todo edge is #{max_todo_count} todo#{max_todo_count > 1 ? 's' : ''}</font>"
100102
end
101103
"<<b>#{main_title}</b>#{sub_title}>"
@@ -150,13 +152,13 @@ def self.node_color_builder
150152
sig { params(all_packages: T::Array[ParsePackwerk::Package], show_edge: T.proc.params(arg0: String, arg1: String).returns(T::Boolean), options: Options).returns(T.nilable(Integer)) }
151153
def self.max_todo_count(all_packages, show_edge, options)
152154
todo_counts = {}
153-
if options.show_todos
155+
if options.show_relationship_todos
154156
all_packages.each do |package|
155157
todos_by_package = package.violations&.group_by(&:to_package_name)
156158
todos_by_package&.keys&.each do |todos_to_package|
157159
todo_types = todos_by_package&& todos_by_package[todos_to_package]&.group_by(&:type)
158160
todo_types&.keys&.each do |todo_type|
159-
if options.only_todo_types.include?(EdgeTodoTypes.deserialize(todo_type))
161+
if options.relationship_todo_types.include?(EdgeTodoTypes.deserialize(todo_type))
160162
if show_edge.call(package.name, todos_to_package)
161163
key = "#{package.name}->#{todos_to_package}:#{todo_type}"
162164
todo_counts[key] = todo_types && todo_types[todo_type]&.count
@@ -215,8 +217,8 @@ def self.filtered(packages, options)
215217

216218
dependents = options.show_dependencies ? dependents_on(packages, focus_pack_name) : []
217219
dependencies = options.show_dependencies ? dependencies_of(packages, focus_pack_name) : []
218-
todos_out = options.show_todos ? todos_out(packages, focus_pack_name, options) : []
219-
todos_in = options.show_todos ? todos_in(packages, focus_pack_name, options) : []
220+
todos_out = options.show_relationship_todos ? todos_out(packages, focus_pack_name, options) : []
221+
todos_in = options.show_relationship_todos ? todos_in(packages, focus_pack_name, options) : []
220222

221223
case options.show_only_edges_to_focus_pack
222224
when FocusPackEdgeDirection::All, FocusPackEdgeDirection::InOut then
@@ -342,7 +344,7 @@ def self.dependents_on(all_packages, focus_packs_names)
342344
def self.todos_in(all_packages, focus_packs_names, options)
343345
all_packages.select do |p|
344346
(p.violations || []).inject([]) do |res, todo|
345-
res << todo.to_package_name if options.only_todo_types.include?(EdgeTodoTypes.deserialize(todo.type))
347+
res << todo.to_package_name if options.relationship_todo_types.include?(EdgeTodoTypes.deserialize(todo.type))
346348
res
347349
end.any? { |v| focus_packs_names.include?(v) }
348350
end.map { |pack| pack.name }
@@ -352,9 +354,43 @@ def self.todos_in(all_packages, focus_packs_names, options)
352354
def self.todos_out(all_packages, focus_packs_names, options)
353355
all_packages.inject([]) do |result, p|
354356
focus_packs_names.include?(p.name) && (p.violations || []).each do |todo|
355-
result << todo.to_package_name if options.only_todo_types.include?(EdgeTodoTypes.deserialize(todo.type))
357+
result << todo.to_package_name if options.relationship_todo_types.include?(EdgeTodoTypes.deserialize(todo.type))
356358
end
357359
result
358360
end
359361
end
362+
363+
sig { params(protection: String, package_name: String, rubocop_config: T.any(NilClass, T::Boolean, T::Hash[String, T.untyped]), rubocop_todo: T.any(NilClass, T::Boolean, T::Hash[String, T.untyped])).returns(T.nilable(Integer)) }
364+
def self.package_based_todos_for(protection, package_name, rubocop_config, rubocop_todo)
365+
rubocop_config = {} if rubocop_config.is_a?(TrueClass) || rubocop_config.is_a?(FalseClass) || rubocop_config.is_a?(NilClass)
366+
rubocop_todo = {} if rubocop_todo.is_a?(TrueClass) || rubocop_todo.is_a?(FalseClass) || rubocop_todo.is_a?(NilClass)
367+
368+
raise ArgumentError unless ['Packs/ClassMethodsAsPublicApis', 'Packs/DocumentedPublicApis', 'Packs/RootNamespaceIsPackName', 'Packs/TypedPublicApis'].include?(protection)
369+
return nil unless (rubocop_config.dig(protection)&.dig('Enabled'))
370+
371+
(rubocop_todo.dig(protection)&.dig('Exclude') || []).inject(0) do |result, todo|
372+
result += 1 if todo.start_with?("#{package_name}/")
373+
result
374+
end
375+
end
376+
377+
sig { returns(T.untyped) }
378+
def self.package_based_todos_text_maker
379+
->(package_name) {
380+
[
381+
'Packs/ClassMethodsAsPublicApis',
382+
'Packs/DocumentedPublicApis',
383+
'Packs/RootNamespaceIsPackName',
384+
'Packs/TypedPublicApis'
385+
].map do |protection|
386+
rubocop_config = File.exist?("#{package_name}/.rubocop.yml") ? YAML.load_file("#{package_name}/.rubocop.yml") : {}
387+
rubocop_todo = File.exist?(".rubocop_todo.yml") ? YAML.load_file(".rubocop_todo.yml") : {}
388+
389+
todo_value = package_based_todos_for(protection, package_name, rubocop_config, rubocop_todo)
390+
abbreviation = T.must(protection.split('/')[1]).chars[0]
391+
392+
"#{abbreviation}: #{todo_value}" if todo_value
393+
end.compact.join(", ")
394+
}
395+
end
360396
end

‎lib/visualize_packs/options.rb‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,12 @@ class Options < T::Struct
3030
prop :show_layers, T::Boolean, default: true
3131
prop :show_visibility, T::Boolean, default: true
3232

33-
prop :show_todos, T::Boolean, default: true
34-
prop :only_todo_types, T::Array[EdgeTodoTypes], default: EdgeTodoTypes.values
35-
prop :use_todos_for_layout, T::Boolean, default: false
33+
prop :show_relationship_todos, T::Boolean, default: true
34+
prop :relationship_todo_types, T::Array[EdgeTodoTypes], default: EdgeTodoTypes.values
35+
prop :use_relationship_todos_for_layout, T::Boolean, default: false
3636

3737
prop :show_teams, T::Boolean, default: true
38+
prop :show_node_todos, T::Boolean, default: true
3839

3940
prop :focus_pack, T.nilable(T::Array[String]), default: nil
4041
prop :show_only_edges_to_focus_pack, FocusPackEdgeDirection, default: FocusPackEdgeDirection::All

‎spec/diagram_generation_helper.sh‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,16 @@ test_names+=("no_legend"); test_params[${test_names[-1]}]="--no-legend"
1919
test_names+=("no_layers"); test_params[${test_names[-1]}]="--no-layers"
2020
test_names+=("no_teams"); test_params[${test_names[-1]}]="--no-teams"
2121

22-
test_names+=("no_dependencies"); test_params[${test_names[-1]}]="--no-dependency-arrows --use-todos-for-layout"
23-
test_names+=("no_todos"); test_params[${test_names[-1]}]="--no-todo-arrows"
24-
test_names+=("no_privacy"); test_params[${test_names[-1]}]="--no-privacy-boxes"
22+
test_names+=("no_dependencies"); test_params[${test_names[-1]}]="--no-dependency-arrows --use-edge-todos-for-layout"
23+
test_names+=("no_todos"); test_params[${test_names[-1]}]="--no-todo-edges"
24+
test_names+=("no_privacy"); test_params[${test_names[-1]}]="--no-privacy-boxes --no-node-todos"
2525
test_names+=("no_nested"); test_params[${test_names[-1]}]="--no-nesting-arrows"
2626
test_names+=("no_visibility"); test_params[${test_names[-1]}]="--no-visibility-arrows"
2727

28-
test_names+=("only_todo_types"); test_params[${test_names[-1]}]="--only-todo-types=architecture,visibility"
28+
test_names+=("relationship_todo_types"); test_params[${test_names[-1]}]="--edge-todo-types=architecture,visibility"
2929
test_names+=("roll_nested_into_parent_packs"); test_params[${test_names[-1]}]="--roll-nested-into-parent-packs"
30-
test_names+=("only_layers"); test_params[${test_names[-1]}]="--no-dependency-arrows --no-todo-arrows --no-privacy-boxes --no-teams --no-nesting-arrows"
31-
test_names+=("no_to_all"); test_params[${test_names[-1]}]="--title='Hide everything' --no-layers --no-visibility --no-dependency-arrows --no-todo-arrows --no-privacy-boxes --no-teams --no-nesting-arrows"
30+
test_names+=("only_layers"); test_params[${test_names[-1]}]="--no-dependency-arrows --no-todo-edges --no-privacy-boxes --no-teams --no-nesting-arrows"
31+
test_names+=("no_to_all"); test_params[${test_names[-1]}]="--title='Hide everything' --no-layers --no-visibility --no-dependency-arrows --no-todo-edges --no-privacy-boxes --no-teams --no-nesting-arrows"
3232
test_names+=("focussed_on_packs_ui"); test_params[${test_names[-1]}]="--focus-pack=packs/ui"
3333
test_names+=("focussed_on_packs_ui_focus_edges"); test_params[${test_names[-1]}]="--focus-pack=packs/ui --focus-pack-edge-mode=inout"
3434
test_names+=("focussed_on_packs_ui_focus_edges_in"); test_params[${test_names[-1]}]="--focus-pack=packs/ui --focus-pack-edge-mode=in"

‎spec/sample_app1/.rubocop_todo.yml‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# This configuration was generated by
2+
# `rubocop --auto-gen-config`
3+
# on 2023-09-25 11:44:46 UTC using RuboCop version 1.56.0.
4+
# The point is for the user to remove these configuration records
5+
# one by one as the offenses are removed from the code base.
6+
# Note that changes in the inspected code, or installation of new
7+
# versions of RuboCop, may require this file to be generated again.
8+
9+
# Offense count: 3
10+
# Configuration parameters: AcceptableParentClasses, AcceptableMixins.
11+
# AcceptableParentClasses: T::Enum, T::Struct, Struct, OpenStruct
12+
Packs/ClassMethodsAsPublicApis:
13+
Exclude:
14+
- 'packs/models/packs/model_a/something'
15+
- 'packs/utility/app/utilities/something'
16+
17+
# Offense count: 1
18+
Packs/DocumentedPublicApis:
19+
Exclude:
20+
- 'packs/models/packs/model_ab/something'
21+
22+
# Offense count: 1
23+
Packs/RootNamespaceIsPackName:
24+
Exclude:
25+
- 'packs/models/packs/model_a/something'
26+
- 'packs/utility/app/utilities/something'
27+
28+
# Offense count: 1
29+
# This cop supports safe autocorrection (--autocorrect).
30+
Packs/TypedPublicApis:
31+
Exclude:
32+
- 'packs/models/packs/model_ab/something'

‎spec/sample_app1/Gemfile.lock‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
PATH
22
remote: ../..
33
specs:
4-
visualize_packs (0.5.20)
4+
visualize_packs (0.5.21)
55
packs-specification
66
parse_packwerk (>= 0.20.0)
77
sorbet-runtime

‎spec/sample_app1/packs/models/.rubocop.yml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
inherit_from: ../../.rubocop.yml
2+
3+
Packs/DocumentedPublicApis:
4+
Enabled: false
5+
6+
Packs/TypedPublicApis:
7+
Enabled: false
8+
9+
Packs/RootNamespaceIsPackName:
10+
Enabled: false
11+
12+
Packs/ClassMethodsAsPublicApis:
13+
Enabled: false

0 commit comments

Comments
 (0)