Skip to content

Commit 1af1a92

Browse files
transclaude
andcommitted
Stop changing what Ruby's own methods do
A new rule, now in CONTRIBUTING.md: a Facets method may share a name with one of Ruby's only if it gives Ruby's results and adds something useful. Otherwise Facets gets out of Ruby's way. Applying it on Ruby 3.4 and 4.0: - Hash#count gives Ruby's answers again. Facets' version counted values instead of key-value pairs. Use hash.values.count(value) for that. - Array#shelljoin (facets/shellwords) is Ruby's again. Facets' version flattened nested arrays and turned hashes into options, so a value meant as one argument, such as a web request's array parameter, could become several. Use array.shellwords.shelljoin to get flags. Both warn until 2027-09-30 when called the way Facets' versions were. - Zlib.inflate and Zlib.deflate are no longer redefined as copies of Ruby's own. - OpenStruct#initialize calls Ruby's own and adds only the block form. Kept, as compatible additions: Kernel#extend with a block (now a module prepended to Kernel, so no -w warning), Pathname#glob with symbol flags, Binding#caller and friends, Pathname#readline, Date#<=> and Time#<=>. Bugs fixed along the way: - facets/fileutils/ln_r replaced FileUtils.link_entry and an internal method with old copies, so Ruby's FileUtils.cp_lr raised NameError. ln_r now uses Ruby's link_entry. - Pathname#glob ignored a block, and with symbol flags didn't use Ruby's glob, so it failed in a directory with glob characters in its name. - Kernel#extend with no arguments and no block returned the object instead of raising ArgumentError, as Ruby's does. - facets/digest required a file that doesn't exist. - facets/zlib didn't require zlib or stringio. - Hash#shellwords put quotes around values, which shelljoin then passed as part of the value. It now gives plain words like --name=jim. Tests can check deprecation warnings with etc/warning_recorder.rb. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 96f2531 commit 1af1a92

21 files changed

Lines changed: 552 additions & 139 deletions

File tree

‎CONTRIBUTING.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,16 @@
2727
require line, and comment out their line in the aggregator file with
2828
`# uncommon`.
2929

30+
* Facets must not change what Ruby's own methods do, in the core or the
31+
standard library. A Facets method with the same name as one of Ruby's is
32+
allowed only if it is compatible, giving Ruby's result for everything
33+
Ruby's version accepts (or differing only where Ruby's gives no useful
34+
answer, such as raising or returning nil), and it adds something useful.
35+
Otherwise Facets gets out of Ruby's way, and the Facets version is either
36+
dropped or kept under a new name. Never redefine a method just to copy
37+
Ruby's. If Ruby later adds a method Facets already has, the same rule
38+
applies.
39+
3040
* Standard libraries that are not extensions of existing standard libraries
3141
do not have to be divvied up into individual method files. But note that
3242
full classes and/or modules are less likely to make it into Facets, as

‎HISTORY.md‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,36 @@ Facets now matches ActiveSupport 8.1 in these cases, which fixes bugs in
99
Rails apps that load Facets, and adds the extra options ActiveSupport's
1010
versions take.
1111

12+
It also stops Facets from changing what Ruby's own methods do. Under a new
13+
rule, now in CONTRIBUTING.md, a Facets method may share a name with one of
14+
Ruby's only if it gives Ruby's results and adds something useful. Otherwise
15+
Facets gets out of Ruby's way.
16+
1217
Changes:
1318

1419
* Bug Fixes
1520

21+
* `Hash#count`, loaded by `require 'facets'`, gives Ruby's own answers
22+
again. Facets' version counted values rather than key-value pairs, which
23+
changed Ruby's answer for every library in the program:
24+
`{a: 1}.count([:a, 1])` returned 0. To count values, use
25+
`hash.values.count(value)`. Until 2027-09-30, `Hash#count` warns when
26+
given an argument that can't be a key-value pair, which is how Facets'
27+
version was called.
28+
* `Array#shelljoin` from `facets/shellwords` is Ruby's own again. Facets'
29+
version flattened nested arrays and turned hashes into options, so a
30+
value meant as one argument could become several. A web request's
31+
parameter can be an array (`?term[]=&term[]=/home`), and with it
32+
`['grep', '-r', '--', term, 'public/'].shelljoin` gave
33+
`grep -r -- '' /home public/`, which prints every file under `/home`.
34+
Ruby's escapes each element as one argument. To turn hashes into
35+
options, use `array.shellwords.shelljoin`. Until 2027-09-30,
36+
`Array#shelljoin` warns when the array has a hash or an array in it.
37+
* `Hash#shellwords`, and so `Hash#shelljoin` and `Array#shellwords`, no
38+
longer put quote characters around values. Once the words were escaped
39+
with `shelljoin`, the quotes became part of the value:
40+
`{name: 'jim'}.shelljoin` passed `"jim"`, quotes included. Each flag and
41+
its value is now one plain word, `--name=jim`.
1642
* `Time.current` uses ActiveSupport's `Time.zone` when it is set. Before,
1743
when Facets loaded after ActiveSupport, `Time.current` ignored the Rails
1844
app's time zone and returned the system's local time.
@@ -34,6 +60,29 @@ Changes:
3460
`"ohaie\nwurld"`. It also raised `NoMethodError` for an empty string.
3561
* `String#indent` with a negative number and an indent string of more than
3662
one character removes only whole copies of that string.
63+
* `require 'facets/fileutils/ln_r'` replaced Ruby's `FileUtils.link_entry`
64+
and an internal method it uses with old copies, so Ruby's own
65+
`FileUtils.cp_lr` raised `NameError`. `FileUtils.ln_r` now uses Ruby's
66+
`link_entry`, and the file requires `fileutils` itself.
67+
* `Pathname#glob` from `facets/pathname/glob` ignored a block; it now gives
68+
the block each match, as Ruby's does. With symbol flags, such as
69+
`:dotmatch`, it now uses Ruby's glob too, so it works in a directory
70+
whose name has glob characters such as `[`, and returns paths in the
71+
same form as it does without them. The file requires `pathname` itself.
72+
* `Kernel#extend` with no arguments and no block raises `ArgumentError`,
73+
as Ruby's does; before, it returned the object. The block form is now a
74+
module prepended to `Kernel`, so `ruby -w` no longer warns that `extend`
75+
was redefined.
76+
* `require 'facets/digest'` raised `LoadError`, because it required a file
77+
that had moved. `#salted_digest`'s automatic salt now comes from
78+
`SecureRandom` instead of `rand`.
79+
* `facets/zlib` no longer defines its own copies of Ruby's `Zlib.inflate`
80+
and `Zlib.deflate`. It now requires `zlib` and `stringio` itself;
81+
before, `Zlib.compress` raised `NameError` unless they were already
82+
loaded.
83+
* `OpenStruct#initialize` from `facets/ostruct` calls Ruby's own
84+
`initialize` for the hash, instead of a copy of it, and `ruby -w` no
85+
longer warns that it was redefined.
3786

3887
* Additions
3988

@@ -48,6 +97,12 @@ Changes:
4897

4998
* Changes that can affect existing code
5099

100+
* `Hash#count(value)` and `Array#shelljoin` no longer do what Facets'
101+
versions did; see Bug Fixes. Both warn until 2027-09-30 when called
102+
the old way.
103+
* `Hash#shellwords` gives `--name=jim` rather than `--name="jim"`. Code
104+
that joined the words with spaces into a shell command should use
105+
`shelljoin` instead.
51106
* `Array#split` keeps an empty piece where a match is at either end:
52107
`[2, 1, 2].split(2)` is `[[], [1], []]`, and `[].split(2)` is `[[]]`.
53108
Before, empty pieces at the end were dropped.

‎demo/standard/shellwords.md‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
## Shellwords
2+
3+
require 'facets/shellwords'
4+
5+
### Hash#shellwords
6+
7+
Turns a hash into command line flags, one word per flag. A one-letter key
8+
gives a short flag and a longer key a long one. A value of `true` gives the
9+
bare flag, and `false` or `nil` leaves the flag out.
10+
11+
{v: true, debug: true, quiet: false}.shellwords.assert == ['-v', '--debug']
12+
13+
Any other value is joined to its flag with `=`, and an array value repeats
14+
the flag for each element.
15+
16+
{name: 'jim', I: ['lib', 'test']}.shellwords.assert ==
17+
['--name=jim', '-I=lib', '-I=test']
18+
19+
The values aren't quoted. Each flag and its value is one word, ready to pass
20+
to `system(*words)`.
21+
22+
{name: 'a b'}.shellwords.assert == ['--name=a b']
23+
24+
### Hash#shelljoin
25+
26+
Escapes the flags and joins them into one string for a shell.
27+
28+
{debug: true, name: 'a b'}.shelljoin.assert == '--debug --name\=a\ b'
29+
30+
The shell splits that string back into the same words.
31+
32+
Shellwords.split('--debug --name\=a\ b').assert == ['--debug', '--name=a b']
33+
34+
### Array#shellwords
35+
36+
Turns an array in the shape of Ruby method arguments, with options in a hash,
37+
into command line words. The flags come first.
38+
39+
['a', 'b', {d: true}].shellwords.assert == ['-d', 'a', 'b']
40+
41+
To make a command line, join the words with Ruby's own `Array#shelljoin`.
42+
43+
['a b', {name: 'jim'}].shellwords.shelljoin.assert == '--name\=jim a\ b'
44+
45+
`Array#shelljoin` itself is Ruby's. It escapes each element as one word and
46+
doesn't turn hashes into flags, since a value meant as one argument must
47+
never become several.

‎etc/test.rb‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
require 'lemon'
55
require 'fileutils'
66
require_relative 'quiet_deprecations'
7+
require_relative 'warning_recorder'
78

89
$:.unshift 'lib/rails'
910
$:.unshift 'lib/core'

‎etc/warning_recorder.rb‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# Lets a test see the warnings a block raises, including the deprecation
2+
# warnings that etc/quiet_deprecations.rb would hide.
3+
#
4+
# warnings = WarningRecorder.record { {a: 1}.count(1) }
5+
# warnings.size.assert == 1
6+
#
7+
# Loaded by etc/test.rb, after quiet_deprecations.rb, so it sees each
8+
# warning first.
9+
module WarningRecorder
10+
# Run the block and return the warning messages it raised, without
11+
# printing them.
12+
def self.record
13+
@messages = []
14+
yield
15+
@messages
16+
ensure
17+
@messages = nil
18+
end
19+
20+
def self.messages
21+
@messages
22+
end
23+
24+
def warn(message, *args, **opts)
25+
messages = WarningRecorder.messages
26+
return super unless messages
27+
messages << message
28+
nil
29+
end
30+
end
31+
32+
Warning.extend(WarningRecorder)

‎lib/core/facets/hash/count.rb‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,27 @@
11
class Hash
22

3-
# Like Enumerable#count, but can count hash values.
3+
# Ruby's own Hash#count, from Enumerable, counts key-value pairs:
44
#
5-
# {:A=>1, :B=>1}.count(1) #=> 2
5+
# {:A=>1, :B=>1}.count([:A, 1]) #=> 1
66
#
7-
def count(*value)
8-
if value.empty?
9-
super()
10-
else
11-
values.count(*value)
7+
# Facets' version used to count values instead, which changed Ruby's
8+
# answer for every library in the program. It now returns Ruby's answer,
9+
# and warns when given something that can't be a key-value pair, since
10+
# that's how the old Facets version was called. To count values, use
11+
# `hash.values.count(value)`.
12+
#
13+
# {:A=>1, :B=>1}.values.count(1) #=> 2
14+
#
15+
# @deprecated Facets no longer changes Hash#count. This warning wrapper is
16+
# scheduled for removal after 2027-09-30.
17+
#
18+
def count(*args, &block)
19+
if args.size == 1 && !(Array === args.first && args.first.size == 2)
20+
warn "Facets' Hash#count(value) is deprecated. Hash#count now counts key-value pairs, " \
21+
"as Ruby's own does, so this returns 0. To count values, use hash.values.count(value). " \
22+
"It will be removed after 2027-09-30.", uplevel: 1
1223
end
24+
super
1325
end
1426

1527
end
16-

‎lib/core/facets/kernel/extend.rb‎

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,36 @@
1-
module Kernel
1+
module Facets
22

3-
extend_method = instance_method(:extend)
3+
# Prepended to Kernel, so it wraps Ruby's own #extend instead of
4+
# replacing it.
5+
module KernelExtend
6+
7+
# Extend an object with a module.
8+
#
9+
# If a block is given, it will be evaluated into an anonymous
10+
# module and used to extend the object.
11+
#
12+
# obj = Object.new
13+
#
14+
# obj.extend do
15+
# def foo; "foo"; end
16+
# end
17+
#
18+
# obj.foo #=> "foo"
19+
#
20+
# Without a block it is Ruby's own #extend.
21+
#
22+
# NOTE: This is one of the few core overrides in Facets.
23+
#
24+
# @author Trans
25+
# @author Marco Otte-Witte (bug fix)
26+
def extend(*mods, &blk)
27+
return super unless blk
28+
super(*mods) unless mods.empty?
29+
super(Module.new(&blk))
30+
end
431

5-
# Extend an object with a module.
6-
#
7-
# If a block is given, it will be evaluated into an anonymous
8-
# module and used to extend the object.
9-
#
10-
# obj = Object.new
11-
#
12-
# obj.extend do
13-
# def foo; "foo"; end
14-
# end
15-
#
16-
# obj.foo #=> "foo"
17-
#
18-
# NOTE: This is one of the few core overrides in Facets.
19-
#
20-
# @author Trans
21-
# @author Marco Otte-Witte (bug fix)
22-
define_method :extend do |*mod, &blk|
23-
extend_method.bind(self).call(*mod) unless mod.empty?
24-
extend_method.bind(self).call(Module.new(&blk)) if blk
25-
self
2632
end
2733

2834
end
35+
36+
Kernel.prepend(Facets::KernelExtend)

‎lib/standard/facets/digest/salted_digest.rb‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
11
require 'digest'
2-
require 'facets/string/random_binary'
2+
require 'securerandom'
33

44
module Digest
55

66
module Instance
7+
# The digest of +str+ followed by +salt+, with the salt appended.
8+
# With +salt+ :auto, the salt is +digest_length+ random bytes from
9+
# SecureRandom.
10+
#
711
# CREDIT: Guido De Rosa
812
def salted_digest(str='', salt=:auto)
913
if salt == :auto
10-
salt = String.random_binary(digest_length)
14+
salt = SecureRandom.random_bytes(digest_length)
1115
end
1216
digest(str + salt) + salt
1317
end

‎lib/standard/facets/fileutils/ln_r.rb‎

Lines changed: 7 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
require 'fileutils'
2+
13
module FileUtils
24

3-
unless method_defined?(:ln_r)
5+
unless respond_to?(:ln_r)
46

57
#
68
# Options: noop verbose dereference_root remove_destination
@@ -25,6 +27,9 @@ module FileUtils
2527
# FileUtils.ln_r 'src/.', 'dest' # cp_r('src', 'dest') makes src/dest,
2628
# # but this doesn't.
2729
#
30+
# It is much like Ruby's own FileUtils.cp_lr, and uses Ruby's
31+
# FileUtils.link_entry to make the links.
32+
#
2833
# TODO: Why --remove-destination and not just --force?
2934
def ln_r(src, dest, options = {})
3035
fu_output_message "ln -r#{options[:remove_destination] ? ' --remove-destination' : ''} #{[src,dest].flatten.join ' '}" if options[:verbose]
@@ -37,61 +42,7 @@ def ln_r(src, dest, options = {})
3742
end
3843
module_function :ln_r
3944

40-
OPT_TABLE = {} unless const_defined?(:OPT_TABLE)
41-
OPT_TABLE['ln_r'] = [:noop, :verbose, :dereference_root, :remove_destination]
42-
43-
#
44-
# Hard links a file system entry +src+ to +dest+.
45-
# If +src+ is a directory, this method links its contents recursively.
46-
#
47-
# Both of +src+ and +dest+ must be a path name.
48-
# +src+ must exist, +dest+ must not exist.
49-
#
50-
# If +dereference_root+ is true, this method dereference tree root.
51-
#
52-
# If +remove_destination+ is true, this method removes each destination file before copy.
53-
#
54-
def link_entry(src, dest, dereference_root = false, remove_destination = false)
55-
Entry_.new(src, nil, dereference_root).traverse do |ent|
56-
destent = Entry_.new(dest, ent.rel, false)
57-
File.unlink destent.path if remove_destination && File.file?(destent.path)
58-
ent.link destent.path
59-
end
60-
end
61-
module_function :link_entry
62-
63-
class Entry_ #:nodoc:
64-
def link(dest)
65-
case
66-
when directory?
67-
if !File.exist?(dest) and descendant_diretory?(dest, path)
68-
raise ArgumentError, "cannot link directory %s to itself %s" % [path, dest]
69-
end
70-
begin
71-
Dir.mkdir dest
72-
rescue
73-
raise unless File.directory?(dest)
74-
end
75-
else file?
76-
File.link path(), dest
77-
end
78-
end
79-
80-
unless const_defined?(:DIRECTORY_TERM)
81-
if File::ALT_SEPARATOR
82-
DIRECTORY_TERM = "(?=[/#{Regexp.quote(File::ALT_SEPARATOR)}]|\\z)".freeze
83-
else
84-
DIRECTORY_TERM = "(?=/|\\z)".freeze
85-
end
86-
SYSCASE = File::FNM_SYSCASE.nonzero? ? "-i" : ""
87-
end
88-
89-
unless method_defined?(:descendant_diretory?)
90-
def descendant_diretory?(descendant, ascendant)
91-
/\A(?#{SYSCASE}:#{Regexp.quote(ascendant)})#{DIRECTORY_TERM}/ =~ File.dirname(descendant)
92-
end
93-
end
94-
end
45+
OPT_TABLE['ln_r'] = [:noop, :verbose, :dereference_root, :remove_destination] if const_defined?(:OPT_TABLE) && !OPT_TABLE.frozen?
9546

9647
end
9748

0 commit comments

Comments
 (0)