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

False positive: structure keys reported as variables without scope prefix #197

Closed
mpaluchowski opened this issue Oct 7, 2016 · 10 comments
Closed

Comments

@mpaluchowski
Copy link
Contributor

The following code code will trigger a false positive:

var testStructure = {
    testKey = "testValue"
};

reporting that testKey is "not declared within a var statement", where it's a structure key and hence doesn't need var.

@ryaneberly
Copy link
Contributor

What version are you using?

The latest in the dev branch, and release 0.7.3 don't flag for me.

Full example

 component {

public void function function1() {
    var testStructure = {
        testKey = "testValue"
    };
}
 }

ryaneberly added a commit that referenced this issue Oct 8, 2016
@mjclemente
Copy link

I'm seeing the same issue with the latest master version and the latest dev version that built successfully. When prepping parameters for QueryExecute(), I get an error warning for each param, when it's structured this way:

    var params = {
      claimId = { value = claimDetail.getID(), cfsqltype = "CF_SQL_INTEGER" },
      serialNo = { value = claimDetail.getSerialNo(), cfsqltype = "CF_SQL_VARCHAR" }
    };

I.e.: "Variable claimId is not declared with a var statement. Use var or the local scope, or otherwise clarify the scope".

Happy to provide more information about version used, etc.

ryaneberly added a commit that referenced this issue Oct 22, 2016
@ryaneberly
Copy link
Contributor

I didn't change any related code that I'm aware of, but it works for me on latest dev build

See:
/CFLint/src/test/resources/com/cflint/tests/VarScoper/structure_keys_197.cfc
/CFLint/src/test/resources/com/cflint/tests/VarScoper/structure_keys_197.expected.txt

If it's still broken for you (on dev branch), please provide a full source example.

@mjclemente
Copy link

I can build off the latest dev branch, but when I run cflint on a folder, I get:

Exception in thread "main" java.lang.NullPointerException
    at com.cflint.main.CFLintMain.mergeConfigFileInFilter(CFLintMain.java:439)
    at com.cflint.main.CFLintMain.execute(CFLintMain.java:364)
    at com.cflint.main.CFLintMain.main(CFLintMain.java:252)

Is there something else that I should be doing? Sorry for the bother and thanks for the help.

ryaneberly added a commit that referenced this issue Oct 25, 2016
null proof it
@ryaneberly
Copy link
Contributor

ok, sorry about that. I null proofed that line - though I couldn't duplicate it with

java -jar target\CFLint-0.8.0-all.jar -folder c:\temp

try again?

@mjclemente
Copy link

mjclemente commented Oct 25, 2016

Thanks Ryan! It builds and runs without issue now. I put together a repo with my failing example. It appears that it's actually related to a parsing error, which in turn generates the false positives.

https://github.com/mjclemente/cflintParamStruct

@ryaneberly
Copy link
Contributor

Ah a multiline quoted string. Interesting. Any idea with dialects and versions support that?

@mjclemente
Copy link

I actually don't think it's the multiline string that's throwing it off. I think it's the boolean !len( thing.getName() ) in the param:

name = { "value" = thing.getName(), "cfsqltype" = "CF_SQL_VARCHAR", null = !len( thing.getName() ) }

When I set it to be simply true or false, there are no issues with the parsing/validation.

And actually, I just tested including the pound signs for it, and that also works. That is, this syntax evaluates without issue: #!len( thing.getName() )#

I don't want to make more work for you. Including the pound signs is an acceptable workaround for my issue.

@ryaneberly
Copy link
Contributor

fixed in cfparser
cfparser/cfparser#60

it will trickle out when the next cfparser and cflint releases happen. Until then you can build it yourself.

Thanks for taking time to identify the issue.

@mjclemente
Copy link

@ryaneberly Thanks for all the work you put into cflint!

ryaneberly added a commit that referenced this issue Nov 24, 2016
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

3 participants