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

"clang-format" changes behavior of ServiceObject constructor in nodejs-common #215

Closed
carnesen opened this issue Sep 27, 2018 · 1 comment

Comments

@carnesen
Copy link
Contributor

As a developer, I expect that my reformatter won't change the runtime behavior of my code. Unfortunately under some conditions clang-format injects non-cosmetic whitespace. See this section of code in nodejs-common:

            return (
                // All ServiceObjects need `request`.
                // clang-format off
                !/^request/.test(methodName) &&
                // clang-format on
                // The ServiceObject didn't redefine the method.
                this[methodName] === ServiceObject.prototype[methodName] &&
                // This method isn't wanted.
                !config.methods![methodName]);

If I remove the off/on comments,clang-format produces the following output:

            return (
                // All ServiceObjects need `request`.
                ! / ^request /.test(methodName) &&
                // The ServiceObject didn't redefine the method.
                this[methodName] === ServiceObject.prototype[methodName] &&
                // This method isn't wanted.
                !config.methods![methodName]);

The spaces inserted into the regex change it's runtime behavior.

Note: I'm filing this mostly to help make my case for a switch to Prettier in googleapis/google-cloud-node#2842 . I'll be adding a comment there soon that references this issue. If we switch to tslint + prettier (via tslint-plugin-prettier and tslint-config-prettier), this issue will be fixed "for free".

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

No branches or pull requests

2 participants