Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize paths in deprecation messages #60

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ jobs:
runs-on: ubuntu-latest
name: Ruby ${{ matrix.ruby }} / ${{ matrix.gemfile }}
strategy:
fail-fast: false
matrix:
gemfile: [gemfiles/activesupport_5.2.gemfile, gemfiles/activesupport_6.0.gemfile, gemfiles/activesupport_6.1.gemfile, gemfiles/activesupport_7.0.gemfile, gemfiles/activesupport_edge.gemfile]
ruby: ["2.6", "2.7", "3.0", "3.1"]
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## main (unreleased)

* [#60](https://github.com/Shopify/deprecation_toolkit/pull/60): Normalize paths in deprecation messages. (@sambostock)

## 2.0.0 (2022-03-16)

* [#58](https://github.com/Shopify/deprecation_toolkit/pull/58): Drop support for Ruby < 2.6 & Active Support < 5.2. (@sambostock)
Expand Down
32 changes: 32 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,38 @@ This setting accepts an array of regular expressions. To match on all warnings,
DeprecationToolkit::Configuration.warnings_treated_as_deprecation = [//]
```

### 🔨 `#DeprecationToolkit::Configuration#message_normalizers`

Deprecation Toolkit allows the normalization of deprecation warnings by registering message normalizers.

Out-of-the-box, various path normalizers are included, to ensure that any paths included in deprecation warnings are consistent from machine to machine (see [`lib/deprecation_toolkit/configuration.rb`](lib/deprecation_toolkit/configuration.rb) for details).

### Customizing normalization

Additional normalizers can be added by appending then to the array:

```ruby
DeprecationToolkit::Configuration.message_normalizers << ->(msg) { msg.downcase }
```

Normalizers are expected to respond to `.call`, accepting a `String` and returning a normalized `String`, and are applied in order of registration.

If you wish to normalize a custom path, you can create your own `DeprecationToolkit::PathPrefixNormalizer`:

```ruby
DeprecationToolkit::Configuration.message_normalizers <<
DeprecationToolkit::PathPrefixNormalizer.new(
'/path/to/something',
replacement: 'optional replacement string',
)
```

You may optionally disable any or all normalizers by mutating or replacing the array.

```ruby
DeprecationToolkit::Configuration.message_normalizers = [] # remove all normalizers
```

## RSpec

By default Deprecation Toolkit uses Minitest as its test runner. To use Deprecation Toolkit with RSpec you'll have to configure it.
Expand Down
1 change: 1 addition & 0 deletions lib/deprecation_toolkit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ module DeprecationToolkit
autoload :DeprecationSubscriber, "deprecation_toolkit/deprecation_subscriber"
autoload :Configuration, "deprecation_toolkit/configuration"
autoload :Collector, "deprecation_toolkit/collector"
autoload :PathPrefixNormalizer, "deprecation_toolkit/path_prefix_normalizer"
autoload :ReadWriteHelper, "deprecation_toolkit/read_write_helper"
autoload :TestTriggerer, "deprecation_toolkit/test_triggerer"

Expand Down
29 changes: 29 additions & 0 deletions lib/deprecation_toolkit/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,34 @@ class Configuration
config_accessor(:deprecation_path) { "test/deprecations" }
config_accessor(:test_runner) { :minitest }
config_accessor(:warnings_treated_as_deprecation) { [] }

config_accessor(:message_normalizers) do
normalizers = []

normalizers << PathPrefixNormalizer.new(Rails.root) if defined?(Rails)
normalizers << PathPrefixNormalizer.new(Bundler.root) if defined?(Bundler)
normalizers << PathPrefixNormalizer.new(Dir.pwd)

if defined?(Gem)
Gem.loaded_specs.each_value do |spec|
normalizers << PathPrefixNormalizer.new(spec.bin_dir, replacement: "<GEM_BIN_DIR:#{spec.name}>")
normalizers << PathPrefixNormalizer.new(spec.extension_dir, replacement: "<GEM_EXTENSION_DIR:#{spec.name}>")
normalizers << PathPrefixNormalizer.new(spec.gem_dir, replacement: "<GEM_DIR:#{spec.name}>")
end
normalizers << PathPrefixNormalizer.new(*Gem.path, replacement: "<GEM_PATH>")
end

begin
require "rbconfig"
normalizers << PathPrefixNormalizer.new(
*RbConfig::CONFIG.values_at("prefix", "sitelibdir", "rubylibdir"),
replacement: "<RUBY_INTERNALS>",
)
rescue LoadError
# skip normalizing ruby internal paths
end

normalizers
end
end
end
16 changes: 11 additions & 5 deletions lib/deprecation_toolkit/deprecation_subscriber.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,24 @@ def self.already_attached?
end

def deprecation(event)
message = event.payload[:message]
message = normalize_message(event.payload[:message])

Collector.collect(message) unless deprecation_allowed?(event.payload)
Collector.collect(message) unless deprecation_allowed?(message, event.payload[:callstack])
end

private

def deprecation_allowed?(payload)
def deprecation_allowed?(message, callstack)
allowed_deprecations, procs = Configuration.allowed_deprecations.partition { |el| el.is_a?(Regexp) }

allowed_deprecations.any? { |regex| regex =~ payload[:message] } ||
procs.any? { |proc| proc.call(payload[:message], payload[:callstack]) }
allowed_deprecations.any? { |regex| regex =~ message } ||
procs.any? { |proc| proc.call(message, callstack) }
end

def normalize_message(message)
Configuration
.message_normalizers
.reduce(message) { |message, normalizer| normalizer.call(message) }
end
end
end
41 changes: 41 additions & 0 deletions lib/deprecation_toolkit/path_prefix_normalizer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# frozen_string_literal: true

require "pathname"

module DeprecationToolkit
class PathPrefixNormalizer
attr_reader :path_prefixes, :replacement

def initialize(*path_prefixes, replacement: "")
@path_prefixes = path_prefixes.compact.map do |path_prefix|
raise ArgumentError, "path prefixes must be absolute: #{path_prefix}" unless Pathname.new(path_prefix).absolute?

ending_in_separator(path_prefix)
end.sort_by { |path| -path.length }
@replacement = replacement.empty? ? replacement : ending_in_separator(replacement)
end

def call(message)
message.gsub(pattern, replacement)
end

def to_s
"s#{pattern}#{replacement}"
end

def pattern
# Naively anchor to the start of a path.
# The last character of each segment of a path is likely to match /\w/.
# Therefore, if the preceeding character does not match /w/, we're probably not in in the middle of a path.
# e.g. In a containerized environment, we may be given `/app` as a path prefix (perhaps from Rails.root).
# Given the following path: `/app/components/foo/app/models/bar.rb`,
# we should replace the prefix, producing: `components/foo/app/models/bar.rb`,
# without corrupting other occurences: `components/foomodels/bar.rb`
@pattern ||= /(?<!\w)#{Regexp.union(path_prefixes)}/
end

def ending_in_separator(path)
File.join(path, "")
end
end
end
152 changes: 152 additions & 0 deletions test/deprecation_toolkit/path_prefix_normalizer_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
# frozen_string_literal: true

require "test_helper"

module DeprecationToolkit
class PathPrefixNormalizerTest < ActiveSupport::TestCase
test "can strip a single path prefix" do
prefix = File.join("", "a")
normalizer = PathPrefixNormalizer.new(prefix)

full_path = File.join("", "a", "b", "c")
normalized_path = File.join("b", "c")

assert_equal normalized_path, normalizer.call(full_path)
end

test "can strip multiple path prefixes" do
prefixes = [File.join("", "a"), File.join("", "d")]
normalizer = PathPrefixNormalizer.new(*prefixes)

full_path = File.join("", "a", "b", "c")
normalized_path = File.join("b", "c")

assert_equal normalized_path, normalizer.call(full_path)

full_path = File.join("", "d", "e", "f")
normalized_path = File.join("e", "f")

assert_equal normalized_path, normalizer.call(full_path)
end

test "multiple path prefixes are replaced individually, not in succession" do
prefixes = [File.join("", "a"), File.join("", "d")]
normalizer = PathPrefixNormalizer.new(*prefixes)

full_path = File.join("", "a", "b", "c", "d", "e", "f")
normalized_path = File.join("b", "c", "d", "e", "f")

assert_equal normalized_path, normalizer.call(full_path)
end

test "replacement is customizable" do
prefix = File.join("", "a")
normalizer = PathPrefixNormalizer.new(prefix, replacement: File.join("eh", ""))

full_path = File.join("", "a", "b", "c")
normalized_path = File.join("eh", "b", "c")

assert_equal normalized_path, normalizer.call(full_path)
end

test "path separator is appended to replacement if not present" do
prefix = File.join("", "a")
normalizer = PathPrefixNormalizer.new(prefix, replacement: "eh")

full_path = File.join("", "a", "b", "c")
normalized_path = File.join("eh", "b", "c")

assert_equal normalized_path, normalizer.call(full_path)
end

test "path separator is not appended to empty replacement" do
prefix = File.join("", "a")
normalizer = PathPrefixNormalizer.new(prefix, replacement: "")

full_path = File.join("", "a", "b", "c")
normalized_path = File.join("b", "c")

assert_equal normalized_path, normalizer.call(full_path)
end

test "given prefix is extended to include trailing separator if not present" do
prefix = File.join("", "a")
normalizer = PathPrefixNormalizer.new(prefix)

full_path = File.join("", "a", "b", "c")
normalized_path = File.join("b", "c")

assert_equal normalized_path, normalizer.call(full_path)
end

test "if given prefix already includes trailing separator, it is not duplicated" do
prefix = File.join("", "a", "")
normalizer = PathPrefixNormalizer.new(prefix)

full_path = File.join("", "a", "b", "c")
normalized_path = File.join("b", "c")

assert_equal normalized_path, normalizer.call(full_path)
end

test "only path prefixes are replaced, not infixes or suffixes" do
prefixes = [File.join("", "a"), File.join("", "b"), File.join("", "c")]
normalizer = PathPrefixNormalizer.new(*prefixes)

full_path = File.join("", "a", "b", "c", "c", "b", "a")
normalized_path = File.join("b", "c", "c", "b", "a")

assert_equal normalized_path, normalizer.call(full_path)
end

test "paths are normalized even if in the middle of the given string" do
prefix = File.join("", "a")
normalizer = PathPrefixNormalizer.new(prefix)

original_message = "The code in #{File.join("", "a", "b", "c")} on line 1 is deprecated!"
normalized_message = "The code in #{File.join("b", "c")} on line 1 is deprecated!"

assert_equal normalized_message, normalizer.call(original_message)
end

test "relative paths are not accepted" do
prefix = File.join("a", "", "b")

error = assert_raises(ArgumentError) do
PathPrefixNormalizer.new(prefix)
end

assert_equal "path prefixes must be absolute: #{prefix}", error.message
end

test "multiple paths in given string are normalized" do
prefix = File.join("", "a")
normalizer = PathPrefixNormalizer.new(prefix)

original_message = "There are deprecations in #{File.join("", "a", "b")} and #{File.join("", "a", "c")}"
normalized_message = "There are deprecations in b and c"

assert_equal normalized_message, normalizer.call(original_message)
end

test "matches are as long as possible" do
prefixes = [File.join("", "a"), File.join("", "a", "b")]
normalizer = PathPrefixNormalizer.new(*prefixes)

full_path = File.join("", "a", "b", "c")
normalized_path = "c"

assert_equal normalized_path, normalizer.call(full_path)
end

test "nil prefixes are ignored" do
prefixes = [File.join("", "a"), nil]
normalizer = PathPrefixNormalizer.new(*prefixes)

full_path = File.join("", "a", "b")
normalized_path = "b"

assert_equal normalized_path, normalizer.call(full_path)
end
end
end
Loading