Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added code to remove default arguments from overridden function definition #89

Merged
merged 3 commits into from
Feb 7, 2019
Merged

added code to remove default arguments from overridden function definition #89

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
39f65db
added funtion and code to remove default arguments from override func…
Feb 6, 2019
8f617aa
added Assets/Tests/KotlinTokenizer/overrideargs.swift
Feb 6, 2019
580b604
PR changes
angelolloqui Feb 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Assets/Tests/KotlinTokenizer/overrideargs.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@

class Test {

override fun dostuff(x: Int) {}
}
5 changes: 5 additions & 0 deletions Assets/Tests/KotlinTokenizer/overrideargs.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class Test {
override func dostuff(x:Int = 5) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very minor, but it is good practice to try to keep formatting as close as possibe to the standard accepted Swift. In this case, the space between x and Int. Not a reason to reject the PR, but just a comment (I changed it myself)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes. I started using swift without spaces between label and type in arguments, but switched a while back, but still forget sometimes...

}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great! this is the kind of tests useful. I am going to add a second method with default arguments not overriden to check they are not removed in that case

}

41 changes: 38 additions & 3 deletions Sources/SwiftKotlinFramework/KotlinTokenizer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public class KotlinTokenizer: SwiftTokenizer {
.replacing({ $0.value == "let"},
with: [constant.newToken(.keyword, "val")])
}

open override func tokenize(_ declaration: FunctionDeclaration) -> [Token] {
let attrsTokens = tokenize(declaration.attributes, node: declaration)
let modifierTokens = declaration.modifiers.map { tokenize($0, node: declaration) }
Expand All @@ -35,15 +35,21 @@ public class KotlinTokenizer: SwiftTokenizer {
genericParameterClauseTokens
].joined(token: declaration.newToken(.space, " "))

let signatureTokens = tokenize(declaration.signature, node: declaration)
var signatureTokens = tokenize(declaration.signature, node: declaration)
let bodyTokens = declaration.body.map(tokenize) ?? []

return [
if modifierTokens.contains(where:{$0.value == "override" }) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of checking the modifiers directly, I am going to move it to an extension like the isStatic because it is a handy check to be used by others in future (and safer to check the type directly rather than the text value)

// overridden methods can't have default args in kotlin:
signatureTokens = removeDefaultArgsFromParameters(tokens:signatureTokens)
}
let tokens = [
headTokens,
[declaration.newToken(.identifier, declaration.name)] + signatureTokens,
bodyTokens
].joined(token: declaration.newToken(.space, " "))
.prefix(with: declaration.newToken(.linebreak, "\n"))

return tokens
}

open override func tokenize(_ parameter: FunctionSignature.Parameter, node: ASTNode) -> [Token] {
Expand Down Expand Up @@ -1078,3 +1084,32 @@ public struct InvertedCondition: ASTTokenizable {
public let condition: Condition
}

// function used to remove default arguments from override functions, since kotlin doesn't have them
private func removeDefaultArgsFromParameters(tokens:[Token]) -> [Token] {
var newTokens = [Token]()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am moving this function to a private method inside the main class. There is no real benefit of it since we are not inheriting for now, but just for consistency with the rest.

var removing = false
var bracket = false
for t in tokens {
if removing && t.kind == .startOfScope && t.value == "(" {
bracket = true
}
if bracket && t.kind == .endOfScope && t.value == ")" {
bracket = false
removing = false
continue
}
if t.kind == .symbol && (t.value.contains("=")) {
removing = true
}
if t.kind == .delimiter && t.value.contains(",") {
removing = false
}
if !bracket && removing && t.kind == .endOfScope && t.value == ")" {
removing = false
}
if !removing {
newTokens.append(t)
}
}
return newTokens
}