[GR-18163] Fix EncodingError message

PullRequest: truffleruby/3634

CHANGELOG.md

@@ -69,6 +69,8 @@ Compatibility:
 * Support writing to `RData.dfree` for native extensions (#2830, #2732, #2165, @eregon).
 * Fix `IO#write` and support multiple arguments with different encodings (#2829, @andrykonchin).
 * Fix `Array` methods `reject`, `reject!`, `inject`, `map`, `select`, `each_index` and handle a case when array is modified by a passed block like CRuby does (#2822, andrykonchin, @eregon).
+* Fix `EncodingError` exception message when Symbol has invalid encoding (#2850, @andrykonchin).
+* Raise `EncodingError` at parse time when Hash literal contains a Symbol key with invalid encoding (#2848, @andrykonchin).
 
 Performance:
 

spec/ruby/core/string/shared/to_sym.rb

@@ -67,6 +67,6 @@
     invalid_utf8.should_not.valid_encoding?
     -> {
       invalid_utf8.send(@method)
-    }.should raise_error(EncodingError, /invalid/)
+    }.should raise_error(EncodingError, 'invalid symbol in encoding UTF-8 :"\xC3"')
   end
 end

spec/ruby/language/hash_spec.rb

@@ -177,6 +177,22 @@ def h.to_hash; {:b => 2, :c => 3}; end
     utf8_hash.keys.first.should == usascii_hash.keys.first
     usascii_hash.keys.first.encoding.should == Encoding::US_ASCII
   end
+
+  it "raises an EncodingError at parse time when Symbol key with invalid bytes" do
+    ScratchPad.record []
+    -> {
+      eval 'ScratchPad << 1; {:"\xC3" => 1}'
+    }.should raise_error(EncodingError, 'invalid symbol in encoding UTF-8 :"\xC3"')
+    ScratchPad.recorded.should == []
+  end
+
+  it "raises an EncodingError at parse time when Symbol key with invalid bytes and 'key: value' syntax used" do
+    ScratchPad.record []
+    -> {
+      eval 'ScratchPad << 1; {"\xC3": 1}'
+    }.should raise_error(EncodingError, 'invalid symbol in encoding UTF-8 :"\xC3"')
+    ScratchPad.recorded.should == []
+  end
 end
 
 describe "The ** operator" do

spec/ruby/language/symbol_spec.rb

@@ -96,11 +96,11 @@
     %I{a b #{"c"}}.should == [:a, :b, :c]
   end
 
-  it "with invalid bytes raises an EncodingError at parse time" do
+  it "raises an EncodingError at parse time when Symbol with invalid bytes" do
     ScratchPad.record []
     -> {
       eval 'ScratchPad << 1; :"\xC3"'
-    }.should raise_error(EncodingError, /invalid/)
+    }.should raise_error(EncodingError, 'invalid symbol in encoding UTF-8 :"\xC3"')
     ScratchPad.recorded.should == []
   end
 end

src/main/java/org/truffleruby/core/exception/CoreExceptions.java

@@ -1111,8 +1111,9 @@ public RubyException regexpError(String message, Node currentNode) {
     // Encoding conversion errors.
 
     @TruffleBoundary
-    public RubyException encodingError(String message, Node currentNode) {
+    public RubyException encodingError(Object string, RubyEncoding encoding, Node currentNode) {
         RubyClass exceptionClass = context.getCoreLibrary().encodingErrorClass;
+        String message = StringUtils.format("invalid symbol in encoding %s :%s", encoding, inspect(string));
         RubyString errorMessage = StringOperations.createUTF8String(context, language, message);
 
         return ExceptionOperations.createRubyException(context, exceptionClass, errorMessage, currentNode, null);

src/main/java/org/truffleruby/core/string/StringNodes.java

@@ -2618,7 +2618,7 @@ protected RubySymbol toSymBroken(Object string, boolean preserveSymbol,
                 @Cached RubyStringLibrary strings,
                 @Bind("strings.getTString(string)") AbstractTruffleString tstring,
                 @Bind("strings.getEncoding(string)") RubyEncoding encoding) {
-            throw new RaiseException(getContext(), coreExceptions().encodingError("invalid encoding symbol", this));
+            throw new RaiseException(getContext(), coreExceptions().encodingError(string, encoding, this));
         }
     }
 

src/main/java/org/truffleruby/parser/BodyTranslator.java

@@ -1307,13 +1307,38 @@ public RubyNode visitDStrNode(DStrParseNode node) {
     public RubyNode visitDSymbolNode(DSymbolParseNode node) {
         SourceIndexLength sourceSection = node.getPosition();
 
+        // A special case for a symbol in Hash literal {"key": value}
+        if (node.size() == 1 && node.getLast() instanceof StrParseNode) {
+            final StrParseNode strParseNode = (StrParseNode) node.getLast();
+            final SymbolParseNode symbolParseNode = asSymbol(sourceSection, strParseNode);
+            return visitSymbolNode(symbolParseNode);
+        }
+
         final RubyNode stringNode = translateInterpolatedString(sourceSection, node.getEncoding(), node.children());
 
         final RubyNode ret = StringToSymbolNodeGen.create(stringNode);
         ret.unsafeSetSourceSection(sourceSection);
         return addNewlineIfNeeded(node, ret);
     }
 
+    /** Same as {@link ParserSupport#asSymbol(SourceIndexLength position, ParseNode value)} but without needing a
+     * ParserSupport instance and only for StrParseNode argument. */
+    private SymbolParseNode asSymbol(SourceIndexLength position, StrParseNode value) {
+        final SymbolParseNode symbolParseNode = new SymbolParseNode(position, value.getValue(),
+                value.encoding);
+
+        if (!symbolParseNode.getTString().isValidUncached(symbolParseNode.getRubyEncoding().tencoding)) {
+            final RubyContext context = RubyLanguage.getCurrentContext();
+            throw new RaiseException(
+                    RubyLanguage.getCurrentContext(),
+                    context.getCoreExceptions().encodingError(
+                            symbolParseNode.getTString(),
+                            symbolParseNode.getRubyEncoding(),
+                            null));
+        }
+        return symbolParseNode;
+    }
+
     private RubyNode translateInterpolatedString(SourceIndexLength sourceSection,
             Encoding encoding, ParseNode[] childNodes) {
         final ToSNode[] children = new ToSNode[childNodes.length];

src/main/java/org/truffleruby/parser/parser/ParserSupport.java

@@ -1259,7 +1259,10 @@ private void checkSymbolCodeRange(SymbolParseNode symbolParseNode) {
         if (!symbolParseNode.getTString().isValidUncached(symbolParseNode.getRubyEncoding().tencoding)) {
             throw new RaiseException(
                     RubyLanguage.getCurrentContext(),
-                    getConfiguration().getContext().getCoreExceptions().encodingError("invalid encoding symbol", null));
+                    getConfiguration().getContext().getCoreExceptions().encodingError(
+                            symbolParseNode.getTString(),
+                            symbolParseNode.getRubyEncoding(),
+                            null));
         }
     }