search

octo-technology / sonar-objective-c

Sonar Plugin for Objective C
517 stars 208 forks source link

Clang analyzer issues #51

Open drewcrawford opened 10 years ago

drewcrawford commented 10 years ago

sonar-objectivec plugin should be reporting clang analyzer issues (and warnings for that matter) to sonarqube.

This is easy to pull by using the xcodebuild analyze command.

cyrilpicat commented 10 years ago

that's avery good idea indeed, and it looks reasonably easy.

Thanks for the idea!

drewcrawford commented 10 years ago

@cyrilpicat If you can lay out a basic sketch of what this patch would look like (e.g. what to subclass, how to report these issues back to sonarqube) I'd be happy to work on it.

drewcrawford commented 10 years ago

@cyrilpicat Just a bump that I can take this on with a little direction from you about what the implementation should look like

drewcrawford commented 10 years ago

@cyrilpicat Another bump that I'm willing to take this issue on if I can get some direction for how it would look

cyrilpicat commented 10 years ago

Thanks for your proposition.

I have not used often this analyzer because in my case it never leads to interesting problems after ARC, so I don't know it very well. Do you have a different opinion. It's still useful?

Could you tell me more what kind of problems it could detect? As it's based on the same Analyzer as OCLint (clang), it might give similar violations, or not?

I just run it with my project and the only thing I can see is that it's generating XML .plist files with the warnings, one per analyzed file. That's it? Is there an other type of report (one per project)? Where is this a bit more documented?

drewcrawford commented 10 years ago

The information you're asking about the purpose and the kind of checks in the clang static analyzer is easily googleable.

Really what I was hoping for was a hint for implementing this feature, given that you were already on board in your first comment, but sometime during the 20 intervening days I figured out how to implement the feature without your help.

I'm not interested in having a holy war about the merits of the clang static analyzer. If you're amenable to merging it in, I can send a PR. Otherwise I'll fork.

cyrilpicat commented 10 years ago

Drew, I think there is a misunderstanding here. My message was not to debate, it was to understand. To understand how things work to be able to give you directions. And you asked me the same question just one day before my answer, so I thought you did not start implementing it.

By the way I am contributing to this plugin on my free time, so I would prefer having some good time than reading this kind of answer. I was just trying to help.

Of course it would be better if the plugin went much quicker but hey, look, there are not that many PR not integrated and the plugin has really been built thanks to this kind of contributions, so of course feel free to contribute.

drewcrawford commented 10 years ago

Everybody here is working in their free time.

I can see how from your perspective there are not that many unmerged PRs, etc. However from my perspective I have many outstanding things I'd like to change, and I have deliberately constrained myself to bothering upstream with a small number of things that I think are fairly obvious / uncontroversial to start with. And those "uncontroversial" things have generally been, well, a lot more debatable than I'd hoped going in, given my expectation that I was deliberately selecting easy things. That's not necessarily a bad thing, pushback can make better patches, but it does require a bit of a thick skin on all sides and my sense is that there's not thick skin on all sides here.

The longer I spend in this issue tracker the more I believe that a fork would be in everybody's best interest. I seem to be advocating for more "aggressive" kinds of changes (keeping in mind I have deliberately submitted what I think are tame things here), you seem to be interested in moving more slowly, you seem to be more concerned with maintaining backwards compatibility with 3.x, my comments seem to be raising your blood pressure, etc. Forking goes a long way to solving all of those problems.

I'm not trying to be rude; I'm just being realistic about how the experiment of me trying to work closely with upstream has gone so far. I think it's fair for both of us to be frustrated, and it may be time to take look at whether we're just pulling in incompatible directions here.

mjdetullio commented 8 years ago

Partial list of checks implemented in PR #97