Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 23d2e1225014
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 1d61432ca876
Choose a head ref
  • 2 commits
  • 2 files changed
  • 1 contributor

Commits on Mar 7, 2017

  1. Fixes a number of assertions in mri test_sprintf but not enough to tag

    any tests out.  Mostly, this fixed two things:
    1. Delims need to match in error messages { or <.
    2. non-ascii symbols should display as actual text.
    enebo committed Mar 7, 2017
    Copy the full SHA
    3e3342f View commit details
  2. More complicated logic to follow if last argument is named hash or ju…

    …st a plain hash.
    
    Improved name reporting to match exact strings in MRI a little closer
    Removed tags which now pass.
    enebo committed Mar 7, 2017
    Copy the full SHA
    1d61432 View commit details
Showing with 23 additions and 16 deletions.
  1. +23 −12 core/src/main/java/org/jruby/util/Sprintf.java
  2. +0 −4 test/mri/excludes/TestSprintf.rb
35 changes: 23 additions & 12 deletions core/src/main/java/org/jruby/util/Sprintf.java
Original file line number Diff line number Diff line change
@@ -112,7 +112,13 @@ private static final class Args {
this.rubyObject = rubyObject;
if (rubyObject instanceof RubyArray) {
this.rubyArray = (RubyArray)rubyObject;
this.rubyHash = null;

if (rubyArray.last() instanceof RubyHash) {
this.rubyHash = (RubyHash) rubyArray.pop(rubyArray.getRuntime().getCurrentContext());
} else {
this.rubyHash = null;
}

this.length = rubyArray.size();
} else if (rubyObject instanceof RubyHash) {
// allow a hash for args if in 1.9 mode
@@ -157,14 +163,14 @@ void warning(ID id, String message) {
if (runtime.isVerbose()) runtime.getWarnings().warning(id, message);
}

private IRubyObject getHashValue(ByteList name) {
private IRubyObject getHashValue(ByteList name, char startDelim, char endDelim) {
// FIXME: get_hash does hash conversion of argv and arity check...this is a bit complicated with
// our version. Implement it.
if (rubyHash == null) {
raiseArgumentError("one hash required");
}

checkNameArg(name);
checkNameArg(name, startDelim, endDelim);
RubySymbol nameSym = runtime.newSymbol(name);
IRubyObject object = rubyHash.fastARef(nameSym);

@@ -173,19 +179,24 @@ private IRubyObject getHashValue(ByteList name) {
if (object == null) {
object = rubyHash.getIfNone();
if (object == RubyBasicObject.UNDEF) {
raiseKeyError("key<" + name + "> not found");
raiseKeyError("key" + startDelim + RubyString.newString(runtime, name) + endDelim + " not found");
} else if (rubyHash.hasDefaultProc()) {
object = object.callMethod(runtime.getCurrentContext(), "call", nameSym);
}

if (object.isNil()) throw runtime.newKeyError("key " + nameSym + " not found");
if (object.isNil()) throw runtime.newKeyError("key" + startDelim + nameSym + endDelim + " not found");
}

return object;
}

private IRubyObject getNthArg(int index) {
if (index > length) raiseArgumentError("too few arguments");
if (index > length) {
if (index == length + 1 && rubyHash != null) {
return rubyHash;
}
raiseArgumentError("too few arguments");
}

return rubyArray == null ? rubyObject : rubyArray.eltInternal(index - 1);
}
@@ -233,9 +244,9 @@ private void checkPositionArg(int nthArg) {
}

// MRI: check_name_arg, CHECKNAMEARG
private void checkNameArg(ByteList name) {
if (positionIndex > 0) raiseArgumentError("named " + name + " after unnumbered(" + positionIndex + ")");
if (positionIndex == -1) raiseArgumentError("named " + name + " after numbered");
private void checkNameArg(ByteList name, char startDelim, char endDelim) {
if (positionIndex > 0) raiseArgumentError("named" + startDelim + RubyString.newString(runtime, name) + endDelim + " after unnumbered(" + positionIndex + ")");
if (positionIndex == -1) raiseArgumentError("named" + startDelim + RubyString.newString(runtime, name) + endDelim + " after numbered");

positionIndex = -2;
}
@@ -429,10 +440,10 @@ private static boolean rubySprintfToBuffer(ByteList buf, CharSequence charFormat

if (nameEnd == nameStart) raiseArgumentError(args, ERR_MALFORMED_NAME);
ByteList newName = new ByteList(format, nameStart, nameEnd - nameStart, encoding, false);
if (name != null) raiseArgumentError(args, "named<" + newName + "> after <" + name + ">");
if (name != null) raiseArgumentError(args, "named<" + RubyString.newString(runtime, newName) + "> after <" + RubyString.newString(runtime, name) + ">");
name = newName;
// we retrieve value from hash so we can generate argument error as side-effect.
args.nextObject = args.getHashValue(name);
args.nextObject = args.getHashValue(name, '<', '>');

break;
}
@@ -453,7 +464,7 @@ private static boolean rubySprintfToBuffer(ByteList buf, CharSequence charFormat
if (nameEnd == nameStart) raiseArgumentError(args, ERR_MALFORMED_NAME);

ByteList localName = new ByteList(format, nameStart, nameEnd - nameStart, encoding, false);
buf.append(args.getHashValue(localName).asString().getByteList());
buf.append(args.getHashValue(localName, '{', '}').asString().getByteList());
incomplete = false;

break;
4 changes: 0 additions & 4 deletions test/mri/excludes/TestSprintf.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
exclude :test_binary, "needs investigation"
exclude :test_float, "needs investigation"
exclude :test_float_hex, "needs investigation"
exclude :test_hash, "failing to inspect hash properly using sprintf %p (#2161)"
exclude :test_inf, "needs investigation"
exclude :test_invalid, "needs investigation"
exclude :test_named_typed, "needs investigation"
exclude :test_named_typed_enc, "needs investigation"
exclude :test_named_untyped, "needs investigation"
exclude :test_named_untyped_enc, "needs investigation"
exclude :test_nan, "needs investigation"
exclude :test_rational, "missing special rational handling in sprintf (#2160)"
exclude :test_star, "differing error message somewhere deep in GETNUM macro from MRI and our equivalents"