Review and correct the Java-to-C# type mappings (#134) - #181
Merged
Conversation
Variables declared against a Java collection interface were being bound to a concrete .NET type: Map became Dictionary and Set became HashSet, losing the abstraction the Java source chose. Map now maps to IDictionary and Set to ISet, matching how List already mapped to IList, and Collection, Comparable, Comparator, Iterable, SortedMap/SortedSet and their Navigable variants are mapped alongside them. That change only works if the concrete implementations are mapped too, since those are what `new Foo<>()` resolves to, so HashMap, LinkedHashMap, LinkedHashSet, TreeMap and TreeSet are added. Java's Map.put is also lowered to an index assignment, the way List.set already was; without it a converted map declaration has no usable way to store into itself. Also fills in the boxed primitives (Character, Double, Short, Byte -- to sbyte, as Java's byte is signed), BigDecimal, StringBuffer, Closeable, and the common exceptions: Throwable, ClassCastException, NumberFormatException, IndexOutOfBoundsException and friends. CharSequence, Runnable and Void are deliberately left unmapped. The mapping table cannot see what position a type appears in, and each of those is valid as a base type in Java but not as its natural C# counterpart -- string is sealed, and `Future<void>` does not compile. The integration harness now mirrors the CLI's default usings and references System.Collections.dll, so the new sample compiles and runs the generated C# rather than only asserting on the conversion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #134.
Interfaces were bound to concrete types
The issue's example —
Map<K, V>mapping toDictionary<K, V>instead ofIDictionary<K, V>— turned out to be one of several. Converting a probe file showed variables declared against a Java collection interface were losing the abstraction the Java source chose, even thoughList→IListwas already correct.Mapnow maps toIDictionaryandSettoISet, withCollection,Comparable,Comparator,Iterable,SortedMap/SortedSetand theNavigablevariants mapped alongside them....which required mapping the concrete types too
Fixing only the interfaces would have made output worse, since interfaces are exactly what you cannot instantiate.
new HashMap<>()was previously emittingnew HashMap()— a type that does not exist in .NET — becauseHashMap,LinkedHashMap,LinkedHashSet,TreeMapandTreeSethad no entries at all. Those are added.Map.putis also lowered to an index assignment, the wayList.setalready was. This is not strictly a type mapping, but the integration test would not compile without it: a convertedIDictionaryhad no usable way to store into itself. It reuses the existingList.setlowering, which already discards the same return value.Other commonly-used types filled in
Boxed primitives (
Character,Double,Short, andByte→sbyte, as Java'sbyteis signed),BigDecimal,StringBuffer,Closeable, and the common exceptions:Throwable,ClassCastException,NumberFormatException,IndexOutOfBoundsException,ArrayIndexOutOfBoundsException,NoSuchElementException,OutOfMemoryError,StackOverflowError,InterruptedException,CloneNotSupportedException.Deliberately left unmapped:
CharSequence,Runnable,VoidAll three were added and then removed.
CharSequence→stringbroke an existing test:class Foo implements CharSequencebecame: string, which does not compile becausestringis sealed.Runnable→ActionandVoid→void(illegal inFuture<Void>) fail the same way.The mapping table is position-blind — it cannot tell a variable declaration from a base-type list — so each of these would fix some conversions while breaking others. Handling them needs position-aware conversion rather than a table entry.
Testing
All 415 tests pass. New unit tests cover the collection, simple-type and exception mappings; a new
CollectionTypeMappings.javaintegration resource compiles and runs the generated C# rather than only asserting on the conversion text. To support that, the integration harness now mirrors the CLI's default usings and referencesSystem.Collections.dll(whereSortedDictionary/SortedSetlive).Pre-existing bugs found but not fixed
Both are out of scope here; happy to file them separately.
new HashMap<>()converts tonew Dictionary(), losing<string, int>. The new test resource uses explicit type arguments to work around this.TypeNameParserhandles simple identifiers only, soMap.Entry<K, V>is left as-is. Relatedly,java.*imports emit unusable usings such asusing Java;.🤖 Generated with Claude Code