Skip to content

Commit 4d05ff2

Browse files
committed
Type stats follow renaming
1 parent 3d0ced6 commit 4d05ff2

6 files changed

Lines changed: 257 additions & 30 deletions

File tree

.github/scripts/typing_stats_compare.rb

Lines changed: 54 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,40 @@
77
head_stats = JSON.parse(File.read(ENV["CURRENT_STATS_PATH"]), symbolize_names: true)
88
base_stats = JSON.parse(File.read(ENV["BASE_STATS_PATH"]), symbolize_names: true)
99

10+
# Parse renames from Git.
11+
RENAMED_PATHS = File.readlines(ENV["RENAMED_PATHS_PATH"], chomp: true).each_with_object({}) do |line, result|
12+
_, base_path, head_path = line.split("\t", 3)
13+
result[head_path] = base_path
14+
end
15+
16+
# Normalizes comparison keys so findings follow renames.
17+
# Example: on rename from base "base.rbs" to head "head.rbs", both paths compare as "base.rbs".
18+
def comparison_key(item)
19+
key = item[:comparison_key].dup
20+
# Normalize key as base path.
21+
key[:path] = RENAMED_PATHS.fetch(key[:path], key[:path])
22+
key
23+
end
24+
25+
# Compares keyed findings while preserving duplicate counts.
26+
def multiset_comparison(head_items, base_items)
27+
unmatched_base = base_items.group_by { |item| comparison_key(item) }
28+
added = []
29+
30+
head_items.each do |item|
31+
base_matches = unmatched_base[comparison_key(item)]
32+
if base_matches&.any?
33+
# Consume one matching base finding so duplicate findings are counted independently.
34+
base_matches.shift
35+
else
36+
added << item
37+
end
38+
end
39+
40+
removed = unmatched_base.values.flatten
41+
[added, removed]
42+
end
43+
1044
def format_for_code_block(data)
1145
data.map do |item|
1246
formatted_string = +"#{item[:path]}:#{item[:line]}"
@@ -146,8 +180,10 @@ def ignored_files_summary(head_stats, base_stats)
146180
end
147181

148182
def steep_ignore_summary(head_stats, base_stats)
149-
steep_ignore_added = head_stats[:steep_ignore_comments] - base_stats[:steep_ignore_comments]
150-
steep_ignore_removed = base_stats[:steep_ignore_comments] - head_stats[:steep_ignore_comments]
183+
steep_ignore_added, steep_ignore_removed = multiset_comparison(
184+
head_stats[:steep_ignore_comments],
185+
base_stats[:steep_ignore_comments]
186+
)
151187

152188
create_summary(
153189
added: steep_ignore_added,
@@ -157,10 +193,14 @@ def steep_ignore_summary(head_stats, base_stats)
157193
end
158194

159195
def untyped_methods_summary(head_stats, base_stats)
160-
untyped_methods_added = head_stats[:untyped_methods] - base_stats[:untyped_methods]
161-
untyped_methods_removed = base_stats[:untyped_methods] - head_stats[:untyped_methods]
162-
partially_typed_methods_added = head_stats[:partially_typed_methods] - base_stats[:partially_typed_methods]
163-
partially_typed_methods_removed = base_stats[:partially_typed_methods] - head_stats[:partially_typed_methods]
196+
untyped_methods_added, untyped_methods_removed = multiset_comparison(
197+
head_stats[:untyped_methods],
198+
base_stats[:untyped_methods]
199+
)
200+
partially_typed_methods_added, partially_typed_methods_removed = multiset_comparison(
201+
head_stats[:partially_typed_methods],
202+
base_stats[:partially_typed_methods]
203+
)
164204
total_methods_base = base_stats[:typed_methods_size] + base_stats[:untyped_methods].size + base_stats[:partially_typed_methods].size
165205
total_methods_head = head_stats[:typed_methods_size] + head_stats[:untyped_methods].size + head_stats[:partially_typed_methods].size
166206
typed_methods_percentage_base = (base_stats[:typed_methods_size] / total_methods_base.to_f * 100).round(2)
@@ -180,10 +220,14 @@ def untyped_methods_summary(head_stats, base_stats)
180220
end
181221

182222
def untyped_others_summary(head_stats, base_stats)
183-
untyped_others_added = head_stats[:untyped_others] - base_stats[:untyped_others]
184-
untyped_others_removed = base_stats[:untyped_others] - head_stats[:untyped_others]
185-
partially_typed_others_added = head_stats[:partially_typed_others] - base_stats[:partially_typed_others]
186-
partially_typed_others_removed = base_stats[:partially_typed_others] - head_stats[:partially_typed_others]
223+
untyped_others_added, untyped_others_removed = multiset_comparison(
224+
head_stats[:untyped_others],
225+
base_stats[:untyped_others]
226+
)
227+
partially_typed_others_added, partially_typed_others_removed = multiset_comparison(
228+
head_stats[:partially_typed_others],
229+
base_stats[:partially_typed_others]
230+
)
187231
total_others_base = base_stats[:typed_others_size] + base_stats[:untyped_others].size + base_stats[:partially_typed_others].size
188232
total_others_head = head_stats[:typed_others_size] + head_stats[:untyped_others].size + head_stats[:partially_typed_others].size
189233
typed_others_percentage_base = (base_stats[:typed_others_size] / total_others_base.to_f * 100).round(2)

.github/scripts/typing_stats_compute.rb

Lines changed: 43 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -39,40 +39,63 @@
3939

4040
total_files_size = Dir.glob("#{project.base_dir}/lib/**/*.rb").size
4141

42+
def stats_item(path, line, line_content, comparison_key)
43+
{
44+
path: path.to_s,
45+
line: line,
46+
line_content: line_content,
47+
comparison_key: comparison_key
48+
}.compact
49+
end
50+
51+
# Include the ignore comment and ignored source so line-only moves stay matched.
52+
def ignore_item(path, line, comment_source, ignored_source)
53+
stats_item(path, line, nil, {type: "steep_ignore", path: path.to_s, source: comment_source, ignored_source: ignored_source})
54+
end
55+
56+
# Include the enclosing namespace so equivalent RBS declarations in different owners stay distinct.
57+
def rbs_item(path, line, source, context)
58+
stats_item(path, line, source, {type: "rbs_declaration", path: path.to_s, context: context, source: source})
59+
end
60+
4261
# steep:ignore comments stats
4362
ignore_comments = loader.each_path_in_patterns(datadog_target.source_pattern).each_with_object([]) do |path, result|
44-
buffer = ::Parser::Source::Buffer.new(path.to_s, 1, source: path.read)
63+
source = path.read
64+
source_lines = source.lines
65+
buffer = ::Parser::Source::Buffer.new(path.to_s, 1, source: source)
4566
_, comments = ::Parser::Ruby25.new.parse_with_comments(buffer)
46-
rbs_buffer = ::RBS::Buffer.new(name: path, content: path.read)
67+
rbs_buffer = ::RBS::Buffer.new(name: path, content: source)
4768
comments.each do |comment|
4869
ignore = ::Steep::AST::Ignore.parse(comment, rbs_buffer)
4970
next if ignore.nil? || ignore.is_a?(::Steep::AST::Ignore::IgnoreEnd)
5071

51-
result << {
52-
path: path.to_s,
53-
line: ignore.line
54-
}
72+
ignored_source = source_lines[ignore.line - 1]&.strip
73+
result << ignore_item(path, ignore.line, comment.loc.expression.source, ignored_source)
5574
end
5675
end
5776

58-
def ast_traversal(declarations, result = {})
77+
# Collects declarations with their enclosing RBS namespace for stable comparison keys.
78+
# @param declarations [Array<RBS::AST::Declarations::Base, RBS::AST::Members::Base>]
79+
# @param result [Hash] accumulated methods and other declarations
80+
# @param context [Array<String>] enclosing Ruby namespace, e.g. ["Datadog", "Core"]
81+
def ast_traversal(declarations, result = {}, context = [])
5982
result[:methods] ||= []
6083
result[:others] ||= []
6184
declarations.each do |declaration|
6285
case declaration
6386
when ::RBS::AST::Declarations::Module,
6487
::RBS::AST::Declarations::Class,
6588
::RBS::AST::Declarations::Interface
66-
ast_traversal(declaration.members, result)
89+
ast_traversal(declaration.members, result, context + [declaration.name.to_s])
6790
when ::RBS::AST::Declarations::TypeAlias,
6891
::RBS::AST::Declarations::Constant,
6992
::RBS::AST::Declarations::Global,
7093
::RBS::AST::Members::Var,
7194
::RBS::AST::Members::Attribute
72-
result[:others] << declaration
95+
result[:others] << {declaration: declaration, context: context}
7396
# Only this one does not have a type field
7497
when ::RBS::AST::Members::MethodDefinition
75-
result[:methods] << declaration
98+
result[:methods] << {declaration: declaration, context: context}
7699
end
77100
end
78101
result
@@ -177,7 +200,9 @@ def is_typed?(type, initialize: false)
177200
_, _directives, declarations = ::RBS::Parser.parse_signature(buffer)
178201
filtered_declarations = ast_traversal(declarations)
179202

180-
filtered_declarations[:methods].each do |method|
203+
filtered_declarations[:methods].each do |entry|
204+
method = entry[:declaration]
205+
181206
# Skip definitions with last comment line being `untyped:accept`
182207
if method.comment&.string&.end_with?("untyped:accept\n")
183208
typed_methods_size += 1
@@ -192,13 +217,15 @@ def is_typed?(type, initialize: false)
192217
when :typed
193218
typed_methods_size += 1
194219
when :untyped
195-
untyped_methods << {path: sig_path.to_s, line: method.location.start_line, line_content: method.location.source}
220+
untyped_methods << rbs_item(sig_path, method.location.start_line, method.location.source, entry[:context])
196221
when :partial
197-
partially_typed_methods << {path: sig_path.to_s, line: method.location.start_line, line_content: method.location.source}
222+
partially_typed_methods << rbs_item(sig_path, method.location.start_line, method.location.source, entry[:context])
198223
end
199224
end
200225

201-
filtered_declarations[:others].each do |declaration|
226+
filtered_declarations[:others].each do |entry|
227+
declaration = entry[:declaration]
228+
202229
# Skip definitions with last comment line being `untyped:accept`
203230
if declaration.comment&.string&.end_with?("untyped:accept\n")
204231
typed_others_size += 1
@@ -209,9 +236,9 @@ def is_typed?(type, initialize: false)
209236
when :typed, nil
210237
typed_others_size += 1
211238
when :untyped
212-
untyped_others << {path: sig_path.to_s, line: declaration.location.start_line, line_content: declaration.location.source}
239+
untyped_others << rbs_item(sig_path, declaration.location.start_line, declaration.location.source, entry[:context])
213240
when :partial
214-
partially_typed_others << {path: sig_path.to_s, line: declaration.location.start_line, line_content: declaration.location.source}
241+
partially_typed_others << rbs_item(sig_path, declaration.location.start_line, declaration.location.source, entry[:context])
215242
end
216243
end
217244
end

.github/workflows/typing-stats.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ jobs:
4444
- name: Checkout current branch
4545
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
4646
with:
47+
fetch-depth: 0
4748
persist-credentials: false
4849

4950
- name: Set up Ruby
@@ -67,6 +68,10 @@ jobs:
6768
mkdir -p "${{ github.workspace }}/tmp"
6869
ruby ${{ runner.temp }}/typing_stats_compute.rb >> "${{ runner.temp }}/typing-stats-current.json"
6970
71+
- name: Collect renamed paths
72+
# Lets the compare script match findings across file renames.
73+
run: git diff --find-renames --diff-filter=R --name-status ${{ github.event.pull_request.base.sha }} HEAD >> "${{ runner.temp }}/typing-stats-renamed-paths.txt"
74+
7075
- name: Checkout base branch
7176
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
7277
with:
@@ -84,6 +89,7 @@ jobs:
8489
env:
8590
CURRENT_STATS_PATH: ${{ runner.temp }}/typing-stats-current.json
8691
BASE_STATS_PATH: ${{ runner.temp }}/typing-stats-base.json
92+
RENAMED_PATHS_PATH: ${{ runner.temp }}/typing-stats-renamed-paths.txt
8793
run: ruby ${{ runner.temp }}/typing_stats_compare.rb >> "${{ runner.temp }}/typing-stats-compare.md"
8894

8995
- name: Write comment

Matrixfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
'core-old' => '✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ 3.4 / ✅ 4.0'
99
},
1010
# Rubocop not included in Ruby < 2.7 gemfiles
11-
'custom_cop' => {
11+
'tooling' => {
1212
'' => '❌ 2.5 / ❌ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ 3.4 / ✅ 4.0'
1313
},
1414
'core_with_libdatadog_api' => {

Rakefile

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ end
9090
desc 'Run RSpec'
9191
namespace :spec do
9292
# REMINDER: If adding a new task here, make sure also add it to the `Matrixfile`
93-
task all: [:main, :benchmark, :custom_cop,
93+
task all: [:main, :benchmark, :tooling,
9494
:graphql, :graphql_unified_trace_patcher, :graphql_trace_patcher, :graphql_tracing_patcher,
9595
:rails, :railsredis, :railsredis_activesupport, :railsactivejob,
9696
:elasticsearch, :http, :redis, :sidekiq, :sinatra, :hanami, :hanami_autoinstrument,
@@ -99,7 +99,10 @@ namespace :spec do
9999
desc '' # "Explicitly hiding from `rake -T`"
100100
RSpec::Core::RakeTask.new(:main) do |t, args|
101101
t.pattern = 'spec/**/*_spec.rb'
102+
103+
# Add new entries as new lines, for readable diffs.
102104
t.exclude_pattern = 'spec/**/{appsec/integration,contrib,benchmark,redis,auto_instrument,opentelemetry,open_feature,profiling,error_tracking,rubocop,ai_guard}/**/*_spec.rb,' \
105+
' spec/tooling/**/*_spec.rb,' \
103106
' spec/**/{auto_instrument,opentelemetry,process,ai_guard}_spec.rb,' \
104107
' spec/**/*_rails_spec.rb,' \
105108
' spec/datadog/core/environment/execution_spec.rb,' \
@@ -115,8 +118,12 @@ namespace :spec do
115118
t.rspec_opts = args.to_a.join(' ')
116119
end
117120

118-
RSpec::Core::RakeTask.new(:custom_cop) do |t, args|
119-
t.pattern = 'spec/rubocop/**/*_spec.rb'
121+
desc 'Scripts and dev tools'
122+
RSpec::Core::RakeTask.new(:tooling) do |t, args|
123+
t.pattern = [
124+
'spec/tooling/**/*_spec.rb',
125+
'spec/rubocop/**/*_spec.rb'
126+
]
120127
t.rspec_opts = args.to_a.join(' ')
121128
end
122129

0 commit comments

Comments
 (0)