UFGInsurance / mulint

Mule project linter
MIT License
6 stars 0 forks source link

Common library variable declared but not used false positive #41

Closed TrueWill closed 5 years ago

TrueWill commented 5 years ago

(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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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