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

#460: Ignoring backticks #461

Closed
wants to merge 3 commits into from
Closed

#460: Ignoring backticks #461

wants to merge 3 commits into from

Conversation

kober32
Copy link

@kober32 kober32 commented Oct 12, 2016

#460 Added util function for removing backticks (`) from variables and constants.

Using this function on [constant-naming], [lower-camel-case], and [constant-k-prefix] rules

[constant-naming], [lower-camel-case] and [constant-k-prefix] rules are now evaluated without backticks
Copy link
Member

@adityatrivedi adityatrivedi left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution @kober32! We are delighted to see the community step in to aid Tailor's evolution!


@Test
public void testBacktickEscapedIdentifier() {
// Backticks are not part of the identifier
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could also test inputs like ``, single character strings, and empty strings to exercise all execution paths?

@@ -28,28 +28,29 @@ public LowerCamelCaseListener(Printer printer) {
@Override
public void enterTopLevel(TopLevelContext topLevelCtx) {
List<IdentifierContext> names = DeclarationListener.getVariableNames(topLevelCtx);
names.forEach(ctx -> verifyLowerCamelCase(Messages.VARIABLE + Messages.NAMES, ctx));
names.forEach(ctx -> verifyLowerCamelCase(Messages.VARIABLE + Messages.NAMES, ctx, true));
Copy link
Member

Choose a reason for hiding this comment

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

Can you please add some representative functional tests to ConstantNamingTest.swift and LowerCamelCaseTest.swift?

}

private void verifyLowerCamelCase(String constructType, ParserRuleContext ctx) {
String constructName = ctx.getText();
private void verifyLowerCamelCase(String constructType, ParserRuleContext ctx, Boolean unescapeIdentifier) {
Copy link
Member

Choose a reason for hiding this comment

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

considering unescapeIdentifier is only true for variable and constant names, could you please make an overloaded function verifyLowerCamelCase(constructType, ctx) that calls this function with unescapeIdentifier set to false (so we don't have to keep passing in false).

@kober32
Copy link
Author

kober32 commented Oct 14, 2016

Hi guys. Thanks for your comments. I will update the PR asap.

j.

@adityatrivedi
Copy link
Member

Closing this PR and tracking changes under #477.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants