Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Common library variable declared but not used false positive #41

Closed
TrueWill opened this issue Nov 13, 2018 · 4 comments
Closed

Common library variable declared but not used false positive #41

TrueWill opened this issue Nov 13, 2018 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@TrueWill
Copy link
Contributor

(From @aoathout )
In project policyholder-accounts-process-api, we have a library file that exposes a bunch of reusable functions for the api. When I ran mulint I saw this:
[warning] policyholder-account-process-lib.dwl: common library variable "fm" declared but not used
But, when I look in the dwl file I see it is being used (example of one of the funcs that use it):

%function findFinancialPolicies(this, policyNumber)
   array.toArray(
       fm.valueForKey(array.firstOrDefault(this), "policies")
   ) filter $.policy_number == policyNumber
@TrueWill TrueWill self-assigned this Nov 13, 2018
@TrueWill TrueWill added the bug Something isn't working label Nov 13, 2018
@TrueWill
Copy link
Contributor Author

TrueWill commented Nov 13, 2018

@aoathout The basic fix is trivial, but the underlying issue is much larger.

Maintainers have been splitting the common-dataweave library into multiple files (number-formatter, string-formatter, datetime-formatter, etc.) and marking the functions in common-formatting-lib obsolete. This means there's no longer just one declaration like
%var fm = readUrl("classpath://dw/common-formatting-lib.dwl")
but instead multiple declarations like
%var dt = readUrl("classpath://dw/datetime-formatter.dwl")
Also the old library function naming convention of valueAs... is no longer followed.

This makes parsing the DataWeave files (with regular expressions) much harder.

Mulint also checks for commented-out lines (assuming they're code), and we're seeing more legitimate comments like
// we only want to pull the last bill data from account_billed
which get flagged as false positives.

As all mulint's validateDataWeaveFiles module does is check for commented-out lines and common library variables declared but not used, I think we should remove that module and its checks. We can parse XML files accurately, but not DataWeave - and even if we had a full parser for that (which I'm not finding), it's difficult to distinguish commented-out code from English comments. I don't think it's worth the effort to detect unused variables.

What do you think?

@aoathout
Copy link

I would agree with your findings. We don't have a language parser for dataweave. While I don't like commented out code, some comments are valid and help guide any developer that has to support things. Maybe in the future when we have time we can think more about convention and do validations on dw then.

@TrueWill
Copy link
Contributor Author

My hope is that MuleSoft will come out with an open source lexer/parser for DataWeave, and we can include some sophisticated static analysis in the future.

@TrueWill
Copy link
Contributor Author

Posted a question on the MuleSoft forums: https://forums.mulesoft.com/questions/102877/dataweave-parser.html (pending moderation)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants