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

Think Bin's element with size is a module #172

Closed
pvalsecc opened this issue Nov 10, 2014 · 11 comments
Closed

Think Bin's element with size is a module #172

pvalsecc opened this issue Nov 10, 2014 · 11 comments
Labels

Comments

@pvalsecc
Copy link

Here is some code:

Data = <<?KEEP_ALIVE:16>>.

KEEP_ALIVE is defined like that:

-define(KEEP_ALIVE, 10).

Elvis is a bit lost:

- Don't use macros (like KEEP_ALIVE on line 55) as module names.

Obviously, in the context of a Bin, KEEP_ALIVE is not used as a module name...

@jfacorro
Copy link
Contributor

@pvalsecc The bug is supposed to be fixed in the branch jfacorro.172.macro.in.binary. Could you please confirm this is working for you?

git clone https://github.com/inaka/elvis
git checkout jfacorro.172.macro.in.binary
cd elvis
make
sudo make install
cd /path/to/project
elvis rock

Thanks!

elbrujohalcon pushed a commit that referenced this issue Nov 17, 2014
[Closes #172] Check node type is 'remote'.
@jfacorro
Copy link
Contributor

@pvalsecc The change is now in master if you find that it is still not working for you please reopen this issue. Thanks!

@pvalsecc
Copy link
Author

@jfacorro Sorry, I didn't see your question. Didn't get the email for some reason.

No, your patch is really not solving my issue.

In this example:

Data = <<?KEEP_ALIVE:16>>.

?KEEP_ALIVE is not used as module name at all. Everything that is in front of a colon is not necessarily a module. In the context of a Bin, this is just a value. And in this context a constant is OK.

@pvalsecc
Copy link
Author

Sorry, I don't have permission to reopen an issue.

@igaray igaray reopened this Nov 18, 2014
@jfacorro
Copy link
Contributor

@pvalsecc That sucks 😞. Would it be possible for you to paste the whole function where the binary is used? It's a long shot but maybe the rule is not taking into account something regarding the context where this expression appears.

@pvalsecc
Copy link
Author

For example, look at mqtt_protocol.erl line 77.

@jfacorro
Copy link
Contributor

@pvalsecc Is it possible that the elvis version you have currently installed is outdated? I just ran the version in master in the erlangmqtt project and this is result I got.

screen shot 2014-11-18 at 11 35 40 am

@dvaergiller
Copy link

The problem also arises in the case where the size is a macro. This is interpreted as a macro as function:

Value:?VALUE_SIZE = Binary

@jfacorro
Copy link
Contributor

@pvalsecc I think @dvaergiller addressed the problem for good. The expressions here are not considered as failures so we are closing this issue. Please let us know if you find any problems.

@pvalsecc
Copy link
Author

@jfacorro , look at the elvis.conf file in the erlangmqtt root, I disabled macro_module_names because of this bug.
I'll test it monday to see if there is a new version of Elvis that fixes it.

@jfacorro
Copy link
Contributor

@pvalsecc Oh, I see. I didn't check the elvis.config, sorry about that 😞.

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

No branches or pull requests

5 participants