Skip to content

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Oct 3, 2019

Context: #464 issues 1 and 2.

Kotlin allows hyphens in class/member names. Replace them with underscores for the managed name so we generate valid C# names.

@jpobst jpobst requested a review from jonpryor October 3, 2019 20:05
@jpobst jpobst force-pushed the managedname-hyphens branch from af815ec to f30a430 Compare October 8, 2019 14:40
@jonpryor jonpryor merged commit bbd75c1 into master Oct 9, 2019
@jonpryor jonpryor deleted the managedname-hyphens branch October 9, 2019 02:33
jonpryor pushed a commit that referenced this pull request Oct 9, 2019
Context: #464
Context: 7a309c4

Use of [Kotlin][0] can result in some "weird" method names and
parameter names in Java bytecode, in particular names which contain
spaces:

	// Kotlin
	abstract class Example {
	    public fun `method spaces`(`parameter spaces`: Double) {
	    }
	}

Run that through `kotlinc` and `javap`, and...there's a parameter
name with spaces!

	$ kotlinc -java-parameters hello.kt
	$ javap Example.class
	Classfile Example.class
	...
	public abstract class Example
	...
	  public final void method spaces(double);
	    ...
	    MethodParameters:
	      Name                           Flags
	      parameter spaces


...which is a sure fire way to confuse things.

Update `XmlApiImporter.EnsureValidIdentifier()` so that it uses
`IdentifierValidator.CreateValidIdentifier()` (7a309c4) to turn
potentially invalid strings such as `method spaces` into valid C#
identifiers such as `method_spaces`, which also ensures that method
names and constructor names are appropriately fixed.

Additionally, update `XmlApiImporter.CreateParameter()` so that it
now uses `EnsureValidIdentifier()`, ensuring it is consistent with
method and constructor name correction.

[0]: https://kotlinlang.org
@github-actions github-actions bot locked and limited conversation to collaborators Apr 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants