Enable static analysis using pedantic for client flutter code

[myiremark]

[myiremark]

https://pub.dev/packages/pedantic

[SamMousa]

@myiremark please elaborate a bit on what you mean with this issue!

[myiremark]

I'm not an expert in Dart/Pedantic, so I won't be able to perfectly elaborate on their differences.

We'd like to not just make sure the code is styled consistently and the way we want it written ala linter e.g Pedantic, we'd like to make sure it evaluates as intended securely and avoids common vulnerabilities at minimum.

It looks like Pedantic is actually built into Dart's static analysis tool in their SDK.

https://github.com/dart-lang/sdk/tree/master/pkg/analyzer

I can't speak to its abilities.

One suggestion I'm more familiar with was https://www.sonarqube.org/.

Dart is not immediately available as being a supported language.

Dart's analyzer may provide equivalent capabilities.

Perhaps a flutter expert can.expound?

[dnfield]

Pedantic provides a common set of analysis lints (code style rules) that are "recommended" for larger team projects. It's also possible to just provide custom analysis rules in your analysis_options.yaml, but the advantage of pedantic is that people are likely familiar with it and it's easy to just drop in.

[dnfield]

And it's maintained by the Googler who manages what lints are applied on Google's Dart based projects.

[advayDev1]

I'm a fan of using an opinionated tool chosen by an expert over us spinning our own. Recommend: Go with pedantic.

[SamMousa]

Already a PR open for pedantic btw

[suztomo]

I use this analysis rule "pedantic_mono" that derives from pedantic: https://pub.dev/packages/pedantic_mono

https://github.com/dart-lang/pedantic + https://github.com/flutter/samples

Its strict rules will provide consistent formatting among wide range of contributors.

[SamMousa]

It lacks a bit of documentation though; @suztomo could you summarize the reasoning behind those rules?

[suztomo]

The reasoning is that the more static analysis rules (that Google's Flutter team uses) more consistent the code will become.

While I don't care the reasoning behind each rule; for example, I don't believe constructors should come first (sort_constructors_first), I do think consistency matters, especially when there are many contributors with many backgrounds.

[SamMousa]

I follow your reasoning, are you up for making a PR?

[suztomo]

Yes, aiming for this weekend. Note that my suggestion does not block onboarding to pedantic.

[SamMousa]

Yes, aiming for this weekend. Note that my suggestion does not block onboarding to pedantic.

That's already been merged. Will leave this open for someone else to pick up, be sure to check back here before you start on your implementation!

Thanks!

[SamMousa]

There is already a PR for basic pedantic, that part is good. The addition would the rules in the mono package.

[SamMousa]

@dnfield would love your opinion on adding the rules from the mono package mentioned earlier.

[creativecreatorormaybenot]

@SamMousa I cannot seem to find that PR.

I think that the rules in mono are somewhat arbitrarily chosen by the author. Considering, pedantic is maintained by the Dart team, it seems like a more reasonable choice. Custom rules can also be added manually.

[SamMousa]

I think that the rules in mono are somewhat arbitrarily chosen by the author.

I agree

Considering, pedantic is maintained by the Dart team, it seems like a more reasonable choice. Custom rules can also be added manually.

What's your recommendation? Sticking with default pedantic, with or without adding extra rules? What do you think about the arguments given above?

[creativecreatorormaybenot]

I actually think that pedantic is already fairly strict and some rules like curly_braces_in_flow_control_structures are poorly implemented and should be excluded.

There are also other extended rule sets, like extra_pedantic, which is actually more popular than pedantic_mono, but they both have a low popularity score.

[SamMousa]

I'll discuss this during engineering standup, my recommendation based on your comments will be to use default pedantic for now.

(This is always a bit of personal preference, so I'd rather stick with the standard)

Thanks for adding to the discussion!

[suztomo]

@creativecreatorormaybenot

For my learning, would you be willing to explain the detail of this poor implementation?

curly_braces_in_flow_control_structures are poorly implemented

[creativecreatorormaybenot]

@suztomo Sure, I opened an issue about this https://github.com/dart-lang/site-www/issues/1994. This will probably be more insightful: https://github.com/dart-lang/linter/issues/1763

[creativecreatorormaybenot]

I realized now that the pedantic dependency was recently added: https://github.com/WorldHealthOrganization/app/pull/339

Any insight on why they are not using the latest version?

[dnfield]

It would probably be best to just add or remove lints as the project sees fit beyond pedantic. I am not familiar with pedantic_mono.

[suztomo]

@creativecreatorormaybenot Thank you.

[britannio]

https://github.com/flutter/flutter/blob/master/analysis_options.yaml These are the rules used in the Flutter repo, hopefully we can tighten up on code style soon by copying these in where appropriate.

[advayDev1]

Let's keep issues focused, updated title

[stale[bot]]

This item has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.