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.

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 StringFilters.camelToSnakeCase filter #24

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ _None_

### New Features

_None_
* Added camelToSnakeCase filter.
[Gyuri Grell](https://github.com/ggrell)
[#24](https://github.com/SwiftGen/StencilSwiftKit/pull/24)

### Internal Changes

Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ _TODO: [Write more extension Documentation](https://github.com/SwiftGen/StencilS
* `join`: Deprecated. Will be removed now that the same filter exists in Stencil proper.
* `lowerFirstWord`
* `snakeToCamelCase` / `snakeToCamelCaseNoPrefix`
* `camelToSnakeCase`: Transforms text from camelCase to snake_case. By default it converts to lower case, unless a single optional argument is set to "false", "no" or "0".
* `titlecase`
* `hexToInt`
* `int255toFloat`
Expand Down
1 change: 1 addition & 0 deletions Sources/Environment.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public extension Extension {
registerFilter("lowerFirstWord", filter: StringFilters.lowerFirstWord)
registerFilter("snakeToCamelCase", filter: StringFilters.snakeToCamelCase)
registerFilter("snakeToCamelCaseNoPrefix", filter: StringFilters.snakeToCamelCaseNoPrefix)
registerFilter("camelToSnakeCase", filter: StringFilters.camelToSnakeCase)
registerFilter("titlecase", filter: StringFilters.titlecase)
registerFilter("hexToInt", filter: NumFilters.hexToInt)
registerFilter("int255toFloat", filter: NumFilters.int255toFloat)
Expand Down
48 changes: 48 additions & 0 deletions Sources/Filters.swift
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,54 @@ struct StringFilters {
}
}

/// Converts camelCase to snake_case. Takes an optional Bool argument for making the string lower case,
/// which defaults to true
/// - parameter value: the value to be processed
/// - parameter arguments: the arguments to the function, expecting zero or one argument
/// - returns: the snake case string
/// - throws: FilterError.invalidInputType if the value parameter isn't a string
static func camelToSnakeCase(_ value: Any?, arguments: [Any?]) throws -> Any? {
let toLower = try parseBool(from: arguments, index: 0, required: false) ?? true
guard let string = value as? String else { throw FilterError.invalidInputType }

let snakeCase = try snakecase(string)
if toLower {
return snakeCase.lowercased()
Copy link
Contributor

Choose a reason for hiding this comment

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

There's already a lowercase filter provided by Stencil, so you could already do foo|camelToSnakeCase|lowercase instead of foo|camelToSnakeCase(true).

I think it would be better to keep the lowercasing separate, i.e. let the caller filter thru |lowercase in the template, instead of making that logic in code (and with the risk of repeating lowercasing logic in Swift code).

Besides, it would allow you to simplify this whole PR by not needing this parseBool anymore (I'm surprised that Stencil doesn't provide a way to parse a boolean argument the same way for all filters already btw?).

And I think it would be more understandable to be explicit. Sure you'll lose the "automatic lowercase by default" behavior, that would be unfortunate, but the code would be much simpler (and the risk of regression and inconsistency in the bool argument parsing logic lowered too)

Copy link
Member

@djbe djbe Mar 4, 2017

Choose a reason for hiding this comment

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

We might need the parseBool logic for other filters we want to refactor (snakeToCamelCaseNoPrefix), but it might then be better to leave that for another PR.

Copy link
Author

@ggrell ggrell Mar 9, 2017

Choose a reason for hiding this comment

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

Currently foo|camelToSnakeCase|lowerCase and foo|camelToSnakeCase are equivalent, and I expect that 99% of people will want the default lowercase behavior. You would only need to add a parameter if you wanted snake case with first upper characters, which IMHO will probably be pretty rare. That's why I'd rather keep it as is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough.

}
return snakeCase
}

/// Parses filter arguments for a boolean value, where true can by any one of: "true", "yes", "1", and
/// false can be any one of: "false", "no", "0". If optional is true it means that the argument on the filter is
/// optional and it's not an error condition if the argument is missing or not the right type
/// - parameter arguments: an array of argument values, may be empty
/// - parameter index: the index in the arguments array
/// - parameter required: if true, the argument is required and function throws if missing. If false, returns nil on missing args.
/// - returns: true or false if a value was parsed, or nil if it wasn't able to
static func parseBool(from arguments: [Any?], index: Int, required: Bool = true) throws -> Bool? {
Copy link
Contributor

Choose a reason for hiding this comment

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

As we're gonna keep this parseBool method, there's no reason for it to be only limited to string filters so no reason to scope it in the StringFilters type. May be worth creating a dedicated type for filter helpers to put it in there (or extend one if there's already such concept in Stencil, haven't checked)

Copy link
Member

Choose a reason for hiding this comment

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

@ggrell Any update on these last small items, or would you prefer I take them up?

guard index < arguments.count else {
if required {
throw FilterError.invalidInputType
} else {
return nil
}
}

let boolArg = (arguments[index] as? String ?? "").lowercased()
switch boolArg {
case "false", "no", "0":
return false
case "true", "yes", "1":
return true
default:
if required {
Copy link
Member

Choose a reason for hiding this comment

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

What I meant with my throwing behaviour was more along the lines of, if I pass the parameter someInvalidStuff instead of true or false, that should be considered and thus throw.

I'd change the guard to:

guard index < arguments.count, let boolArg = arguments[index] as? String else {
  ... required check...

In the switch, you then don't check for required, the default case always throws.

Copy link
Author

Choose a reason for hiding this comment

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

Yup, good eye!

throw FilterError.invalidInputType
} else {
return nil
}
}
}

/**
This returns the string with its first parameter uppercased.
- note: This is quite similar to `capitalise` except that this filter doesn't lowercase
Expand Down
8 changes: 8 additions & 0 deletions StencilSwiftKit.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

/* Begin PBXBuildFile section */
82EF0CC0752D216C67279A16 /* Pods_Tests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 8BF798509C76E5A9ACE03491 /* Pods_Tests.framework */; };
B5A3F2ED5DA57C06EF62BB82 /* ParseBoolTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B5A3FFC01B2145C4BFD8316A /* ParseBoolTests.swift */; };
B5A3FE30A2D1C19DCC66F138 /* XCTest+Assertions.swift in Sources */ = {isa = PBXBuildFile; fileRef = B5A3F3C5A04BF1D8B85403EA /* XCTest+Assertions.swift */; };
DD4393FF1E2D3EEB0047A332 /* MapNodeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = DD4393FE1E2D3EEB0047A332 /* MapNodeTests.swift */; };
DD5F341B1E21993A00AEB5DA /* TestsHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = DD5F341A1E21993A00AEB5DA /* TestsHelper.swift */; };
DD5F342E1E21A3A200AEB5DA /* CallNodeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = DD5F342A1E21A3A200AEB5DA /* CallNodeTests.swift */; };
Expand Down Expand Up @@ -57,6 +59,8 @@
47888DD528DEC4C84FD8F15B /* Pods-Tests.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Tests.debug.xcconfig"; path = "Pods/Target Support Files/Pods-Tests/Pods-Tests.debug.xcconfig"; sourceTree = "<group>"; };
4B3D39DBCD15D8F6BB891D92 /* Pods-Tests.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Tests.release.xcconfig"; path = "Pods/Target Support Files/Pods-Tests/Pods-Tests.release.xcconfig"; sourceTree = "<group>"; };
8BF798509C76E5A9ACE03491 /* Pods_Tests.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Tests.framework; sourceTree = BUILT_PRODUCTS_DIR; };
B5A3F3C5A04BF1D8B85403EA /* XCTest+Assertions.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "XCTest+Assertions.swift"; sourceTree = "<group>"; };
B5A3FFC01B2145C4BFD8316A /* ParseBoolTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ParseBoolTests.swift; sourceTree = "<group>"; };
DD4393FE1E2D3EEB0047A332 /* MapNodeTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MapNodeTests.swift; sourceTree = "<group>"; };
DD5F341A1E21993A00AEB5DA /* TestsHelper.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TestsHelper.swift; sourceTree = "<group>"; };
DD5F34201E2199ED00AEB5DA /* Info.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
Expand Down Expand Up @@ -148,6 +152,8 @@
DD5F342D1E21A3A200AEB5DA /* SwiftIdentifierTests.swift */,
DD5F341A1E21993A00AEB5DA /* TestsHelper.swift */,
DD5F341C1E2199ED00AEB5DA /* Resources */,
B5A3FFC01B2145C4BFD8316A /* ParseBoolTests.swift */,
B5A3F3C5A04BF1D8B85403EA /* XCTest+Assertions.swift */,
);
path = StencilSwiftKitTests;
sourceTree = "<group>";
Expand Down Expand Up @@ -284,6 +290,8 @@
DDFD1F691E5358C70023AE2B /* ContextTests.swift in Sources */,
DD5F342E1E21A3A200AEB5DA /* CallNodeTests.swift in Sources */,
DDE1E2F91E3FABE70043367C /* ParametersTests.swift in Sources */,
B5A3F2ED5DA57C06EF62BB82 /* ParseBoolTests.swift in Sources */,
B5A3FE30A2D1C19DCC66F138 /* XCTest+Assertions.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down
77 changes: 77 additions & 0 deletions Tests/StencilSwiftKitTests/ParseBoolTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
//
// StencilSwiftKit
// Copyright (c) 2017 SwiftGen
// MIT Licence
//

import XCTest
@testable import StencilSwiftKit

class ParseBoolTests: XCTestCase {

func testParseBool_WithTrueString() throws {
let value = try StringFilters.parseBool(from: ["true"], index: 0)
XCTAssertTrue(value!)
}

func testParseBool_WithFalseString() throws {
let value = try StringFilters.parseBool(from: ["false"], index: 0)
XCTAssertFalse(value!)
}

func testParseBool_WithYesString() throws {
let value = try StringFilters.parseBool(from: ["yes"], index: 0)
XCTAssertTrue(value!)
}

func testParseBool_WithNoString() throws {
let value = try StringFilters.parseBool(from: ["no"], index: 0)
XCTAssertFalse(value!)
}

func testParseBool_WithOneString() throws {
let value = try StringFilters.parseBool(from: ["1"], index: 0)
XCTAssertTrue(value!)
}

func testParseBool_WithZeroString() throws {
let value = try StringFilters.parseBool(from: ["0"], index: 0)
XCTAssertFalse(value!)
}

func testParseBool_WithOptionalInt() throws {
let value = try StringFilters.parseBool(from: [1], index: 0, required: false)
XCTAssertNil(value)
}

func testParseBool_WithRequiredInt() throws {
XCTAssertThrows(try StringFilters.parseBool(from: [1], index: 0, required: true))
}

func testParseBool_WithOptionalDouble() throws {
let value = try StringFilters.parseBool(from: [1.0], index: 0, required: false)
XCTAssertNil(value)
}

func testParseBool_WithRequiredDouble() throws {
XCTAssertThrows(try StringFilters.parseBool(from: [1.0], index: 0, required: true))
}

func testParseBool_WithEmptyString() throws {
let value = try StringFilters.parseBool(from: [""], index: 0, required: false)
XCTAssertNil(value)
}

func testParseBool_WithEmptyStringAndRequiredArg() throws {
XCTAssertThrows(try StringFilters.parseBool(from: [""], index: 0, required: true))
}

func testParseBool_WithEmptyArray() throws {
let value = try StringFilters.parseBool(from: [], index: 0, required: false)
XCTAssertNil(value)
}

func testParseBool_WithEmptyArrayAndRequiredArg() throws {
XCTAssertThrows(try StringFilters.parseBool(from: [], index: 0, required: true))
}
}
71 changes: 71 additions & 0 deletions Tests/StencilSwiftKitTests/StringFiltersTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,77 @@ class StringFiltersTests: XCTestCase {
}
}

func testCamelToSnakeCase_WithNoArgsDefaultsToTrue() {
Copy link
Member

Choose a reason for hiding this comment

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

You can remove this case, it's covered by the parseBool tests and the 2 cases below this one.

Copy link
Author

@ggrell ggrell Feb 27, 2017

Choose a reason for hiding this comment

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

This test covers the ?? true flow (and validated that if no argument given, it'll default to lower case) in the line:
let toLower = try parseBool(from: arguments, index: 0, required: false) ?? true because there's no other test for the no arguments case. The other two camelToSnakeCase tests pass arguments.

Copy link
Member

Choose a reason for hiding this comment

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

True, good call.

let result = try! StringFilters.camelToSnakeCase("StringWithWords", arguments: []) as? String
XCTAssertEqual(result, "string_with_words")
}

func testCamelToSnakeCase_WithTrue() {
let expectations = [
"string": "string",
"String": "string",
"strIng": "str_ing",
"strING": "str_ing",
"X": "x",
"x": "x",
"SomeCapString": "some_cap_string",
"someCapString": "some_cap_string",
"string_with_words": "string_with_words",
"String_with_words": "string_with_words",
"String_With_Words": "string_with_words",
"String_With_WoRds": "string_with_wo_rds",
"STRing_with_words": "st_ring_with_words",
"string_wiTH_WOrds": "string_wi_th_w_ords",
"": "",
"URLChooser": "url_chooser",
"UrlChooser": "url_chooser",
"a__b__c": "a__b__c",
"__y_z!": "__y_z!",
"PLEASESTOPSCREAMING": "pleasestopscreaming",
"PLEASESTOPSCREAMING!": "pleasestopscreaming!",
"PLEASE_STOP_SCREAMING": "please_stop_screaming",
"PLEASE_STOP_SCREAMING!": "please_stop_screaming!"
]

for (input, expected) in expectations {
let trueArgResult = try! StringFilters.camelToSnakeCase(input, arguments: ["true"]) as? String
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned above, it might be better to separate the boolean parsing logic into it's own method, that can then be tested separately.

XCTAssertEqual(trueArgResult, expected)
}
}

func testCamelToSnakeCase_WithFalse() {
let expectations = [
"string": "string",
Copy link
Contributor

Choose a reason for hiding this comment

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

A little nitpicking, but could you ensure the indentation you use is consistent with the rest of the project? Thx
(Here even in your own code you have different indentation levels)

"String": "String",
"strIng": "str_Ing",
"strING": "str_ING",
"X": "X",
"x": "x",
"SomeCapString": "Some_Cap_String",
"someCapString": "some_Cap_String",
"string_with_words": "string_with_words",
"String_with_words": "String_with_words",
"String_With_Words": "String_With_Words",
"String_With_WoRds": "String_With_Wo_Rds",
"STRing_with_words": "ST_Ring_with_words",
"string_wiTH_WOrds": "string_wi_TH_W_Ords",
"": "",
"URLChooser": "URL_Chooser",
"UrlChooser": "Url_Chooser",
"a__b__c": "a__b__c",
"__y_z!": "__y_z!",
"PLEASESTOPSCREAMING": "PLEASESTOPSCREAMING",
"PLEASESTOPSCREAMING!": "PLEASESTOPSCREAMING!",
"PLEASE_STOP_SCREAMING": "PLEASE_STOP_SCREAMING",
"PLEASE_STOP_SCREAMING!": "PLEASE_STOP_SCREAMING!"
]

for (input, expected) in expectations {
let falseArgResult = try! StringFilters.camelToSnakeCase(input, arguments: ["false"]) as? String
XCTAssertEqual(falseArgResult, expected)
}
}

func testEscapeReservedKeywords() throws {
let expectations = [
"self": "`self`",
Expand Down
11 changes: 11 additions & 0 deletions Tests/StencilSwiftKitTests/XCTest+Assertions.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//
// StencilSwiftKit
// Copyright (c) 2017 SwiftGen
// MIT Licence
//

import XCTest

public func XCTAssertThrows(_ expression: @autoclosure () throws -> Any?, _ message: @autoclosure () -> String = "", file: StaticString = #file, line: UInt = #line) {
XCTAssert((try? expression()) == nil, message, file: file, line: line)
}