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

Invalid function names #1265

Closed
HugoGiraudel opened this Issue Jun 3, 2014 · 8 comments

Comments

Projects
None yet
3 participants
@HugoGiraudel
Contributor

HugoGiraudel commented Jun 3, 2014

A couple of months ago, I opened an issue regarding functions called true(), false() and null() that didn't work as expected.

Today, I found out functions called and(), or() and not() are even worse. Both and() and or() returns a syntax error when called (not declared):

@function and() {
  @return "and"
}

test {
  and: and();
}

Invalid CSS after " and: ": expected expression (e.g. 1px, bold), was "and();"

@function or() {
  @return "or"
}

test {
  or: or();
}

Invalid CSS after " or: ": expected expression (e.g. 1px, bold), was "or();"

The not() function on the other side partially works. It doesn't throw any error but it returns false, no matter what.

@function not() {
  @return "not"
}

test {
  not: not();
}
test {
  not: false;
}

Tested on Sass 3.3.6 and 3.2.19. Curious things is everything works like a charm with LibSass.

@nex3 nex3 added the Bug label Jun 6, 2014

@nex3

This comment has been minimized.

Contributor

nex3 commented Jun 6, 2014

Curious things is everything works like a charm with LibSass.

By this, do you mean that the functions are called or that an error is produced at definition-time? The latter is what I'd consider correct behavior here.

@HugoGiraudel

This comment has been minimized.

Contributor

HugoGiraudel commented Jun 6, 2014

By this, do you mean that the functions are called or that an error is produced at definition-time? The latter is what I'd consider correct behavior here.

LibSass not only allows all 3 functions but also returns expected results from all 3 (respectively "and", "or" and "not").

@akhleung

This comment has been minimized.

akhleung commented Jun 6, 2014

In LibSass, not is actually implemented as an ordinary function (which unfortunately has resulted in an annoying bug where expressions such as not foo don't get evaluated), but I do agree with Nathan that reserved keywords such as and and or shouldn't be allowed as function names, and attempts to define such functions should raise an error.

@HugoGiraudel

This comment has been minimized.

Contributor

HugoGiraudel commented Jun 6, 2014

reserved keywords such as and and or shouldn't be allowed as function names, and attempts to define such functions should raise an error.

Why not. I am not sure why but perhaps there are technical implications regarding this. No matter what, I think it should at least return an explicit error message.

@nex3

This comment has been minimized.

Contributor

nex3 commented Jun 6, 2014

It's a parsing issue. Allowing functions to have operator names makes SassScript like foo and(), foo or(), and not() ambiguous (not to mention backwards-incompatible with the current behavior).

@HugoGiraudel

This comment has been minimized.

Contributor

HugoGiraudel commented Jun 6, 2014

Okay. Out of curiosity, wasn't it the same deal with #1090?

@nex3

This comment has been minimized.

Contributor

nex3 commented Jun 6, 2014

There are technical differences and design differences. The short explanation of the technical side is that operators are parsed pretty differently than values.

In terms of design, operators intuitively feel more like they should separate expressions, so not allowing them to do so in the case that they're followed immediately by parentheses is weird. Values, on the other hand, are often not allowed to exist directly next to one another without any separators, so making that case parse as a function feels more reasonable.

@nex3 nex3 closed this in 9e58d2e Jun 13, 2014

@HugoGiraudel

This comment has been minimized.

Contributor

HugoGiraudel commented Jun 13, 2014

That's nice seeing a fix for that. :)

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