[Csnd-dev] Any interest in the use of c linters?
| Date | 2022-12-14 13:51 |
| From | Hlöðver Sigurðsson |
| Subject | [Csnd-dev] Any interest in the use of c linters? |
The use of linters in javascript I think shows how much value they give to open source projects. In that they can find common fallacies and errors in the code from just analyzing the code. This should also give more confidence into pull request from third party contributors, in that the reviewer doesn't have to bother with simple errors and have the linter do that work.
One example of this in c I found is how the chromium project is linted with clang-format (for formatting) and clang-tidy (for finding errors / linting). https://chromium.googlesource.com/chromium/src.git/+/refs/heads/main/.clang-format https://chromium.googlesource.com/chromium/src.git/+/refs/heads/main/.clang-tidy I assume this could be a somewhat radical change, and we won't need to implement this into github workflows to begin with. But I feel this could give myself personally a way to know if I'm writing code in a modernized way, and writing code that isn't doing something obviously wrong. If using linters in the c codebase is strongly objected to, I'd be fully okay with that. But I could also open a PR for adding and applying these two dotfiles to show the changes it would bring to the code formatting and perhaps change the way some expressions are written, and we could discuss if these changes are something we would be ok with merging. Let me know what you think :) |
| Date | 2022-12-14 14:00 |
| From | Rory Walsh |
| Subject | Re: [Csnd-dev] Any interest in the use of c linters? |
Is this not what the coverity branch is for? Different tool, similar results? On Wed, 14 Dec 2022 at 13:52, Hlöðver Sigurðsson <hlolli@gmail.com> wrote:
|
| Date | 2022-12-14 14:00 |
| From | Dave Seidel |
| Subject | Re: [Csnd-dev] Any interest in the use of c linters? |
As someone who is not on the Csound dev team, but started using C in the 80's and started using lint in the 90s (and similar tools with Java and Python, etc.), I think it's a good idea. But I would warn that you will need to expect that it will take a bit of time and effort to configure it so that it's truly helpful and not just driving you crazy with irrelevant warnings. - Dave On Wed, Dec 14, 2022 at 8:52 AM Hlöðver Sigurðsson <hlolli@gmail.com> wrote:
|
| Date | 2022-12-14 14:22 |
| From | Hlöðver Sigurðsson |
| Subject | Re: [Csnd-dev] Any interest in the use of c linters? |
I agree on that warnings are annoying. So I would rather take an approach to only error, and if an exception must be made, then a comment next to the code block to tell the linter to now bother with a given rule being broken. On Wed, 14 Dec 2022 at 15:03, Dave Seidel <dave.seidel@gmail.com> wrote:
|
| Date | 2022-12-14 14:27 |
| From | Dave Seidel |
| Subject | Re: [Csnd-dev] Any interest in the use of c linters? |
Sounds like a good policy. On Wed, Dec 14, 2022 at 9:23 AM Hlöðver Sigurðsson <hlolli@gmail.com> wrote:
|
| Date | 2022-12-14 22:31 |
| From | Hlöðver Sigurðsson |
| Subject | Re: [Csnd-dev] Any interest in the use of c linters? |
I opened a PR and ran relatively vanilla formatting and linting rules, so I ask the question, are these proposed fixes something we'd like to apply to the csound codebase, should we modify or disable some of the rules? Please take a look at the two following failed workflow runs from my PR suggestions for formatting https://github.com/csound/csound/actions/runs/3699226418/jobs/6266361148 suggestion for tidy/linting https://github.com/csound/csound/actions/runs/3699226415/jobs/6266361443 Besides the radical changes the linters are suggesting, I try to introduce this conservatively, with cmake scripts, which build the two make targets tidy and format only if the required programs for it are found. Pre-commit hooks exist for clang-format, but only if a developer wishes to include it, they can, otherwise it's not enforced. On Wed, 14 Dec 2022 at 15:29, Dave Seidel <dave.seidel@gmail.com> wrote:
|
| Date | 2022-12-15 09:40 |
| From | Victor Lazzarini |
| Subject | Re: [Csnd-dev] [EXTERNAL] Re: [Csnd-dev] Any interest in the use of c linters? |
|
To me the ability to run cmake format etc locally is good. I am not sure about the github actions.
John is the one who does normally run formatting scripts. Maybe he should be the one to say if he's happy with the format chosen.
The linting may be of limited use though. We used the coverity static analysers for years and that was ok, so the code should be reasonably tidy bar false positives.
I think reinstating Coverity would be a good thing if we could.
Prof. Victor Lazzarini
Maynooth University
Ireland
On 14 Dec 2022, at 22:33, Hlöðver Sigurðsson <hlolli@gmail.com> wrote:
|
| Date | 2022-12-15 12:47 |
| From | Hlöðver Sigurðsson |
| Subject | Re: [Csnd-dev] [EXTERNAL] Re: [Csnd-dev] Any interest in the use of c linters? |
I did a little digging on Coverity, and I have no problem with using it. It can run in automation when we publish releases and can give us a nice badge that allows us to see the reports. Since I don't know much about clang-tidy, I wont be pushing for it too hard. But I do hope it would be ok to keep it as a 'make tidy' target for local use, it doesn't have to be running in CI. Because, the suggestions from clang-tidy overlap only slightly with the findings and suggestions from coverity. Clang has a different tool called clang-static-analyzer which more resembles what Coverity is doing. Unlike coverity, clang-tidy is more of a linter in the sense that it will complain about poor code readability and arcane structure. Like coverity, clang-tidy can make suggestions for better memory management and optimization, but I don't think clang-tidy will go as deep as Coverity in that area. Running code formatter in CI is something that can be hard to get used to, but the alternative is that we run the formatter whenever we feel like, that can cause the git diffs to get annoying. Because if one person makes a change that wasn't formatted, and another one formats it, then the latest change from "git blame" will make it look that the person that ran the code formatter is the person that made the change and not the author. So it's always preferable if the person making a change is the one that also ran the formatter. For this reason I also added .pre-commit file, which I would recommend using if we were to use formatter, which will run the formatter whenever making a new commit, to prevent unformatted code from being pushed in the first place. One option for both clang-tidy and clang-format, would be if we run both in github actions, but forcefully make sure that it always succeeds and never shows any red marking, that way it could serve as a suggestion which we can read if we want to, but could choose to ignore it. And once in a while we could take a look to see if these suggestions and apply them or modify the rules. I think that could be a nice step 1, because currently we have nothing in place, and we could have a period of time in getting used to using formatter and perhaps in some years time we could enforce this harder. On Thu, 15 Dec 2022 at 10:40, Victor Lazzarini <Victor.Lazzarini@mu.ie> wrote:
|