Skip to content

misra.py: Fixup rules severity#1911

Merged
danmar merged 2 commits into
cppcheck-opensource:masterfrom
jubnzv:misra-severity
Jun 23, 2019
Merged

misra.py: Fixup rules severity#1911
danmar merged 2 commits into
cppcheck-opensource:masterfrom
jubnzv:misra-severity

Conversation

@jubnzv
Copy link
Copy Markdown
Contributor

@jubnzv jubnzv commented Jun 21, 2019

No description provided.

Comment thread addons/misra.py Outdated
class Rule(object):
"""Class to keep rule text and metadata"""

SEVERITY_MAP = {'Required': 'warning', 'Mandatory': 'error', 'Advisory': 'style', 'style': 'style'}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm.. this was not your idea, but this does not seem right to me. The severity should explain what type of issue is seen. Here, another definition is used.

I think it's better we always say 'style' for misra addon output. I did not want to put many (or all) of these checks in Cppcheck because they are too stylistic.

Copy link
Copy Markdown
Collaborator

@danmar danmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry but I think you were mislead by the old code. I'd like to see a cleanup.

@jubnzv
Copy link
Copy Markdown
Contributor Author

jubnzv commented Jun 23, 2019

I think it's better we always say 'style' for misra addon output. I did not want to put many (or all) of these checks in Cppcheck because they are too stylistic.

Yes, that makes sense.

I print severity levels like this:

[src/foo.c:76] (style) Rule text. (Required) [misra-c2012-21.3]
[src/foo.c:14] (style) Rule text. (Mandatory) [misra-c2012-21.6]

I also show counters in summary using MISRA severity levels:

MISRA rules violations found:
        Undefined: 25
        Required: 18
        Advisory: 12

@danmar danmar merged commit 05bb4a0 into cppcheck-opensource:master Jun 23, 2019
@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Jun 23, 2019

hmm.. I think we need to tweak the GUI and maybe some other stuff also. The "rule text" column should not say "(Required)". If the output is saved in xml format I would also like that it is not saved in the warning message but kept separately. I signed a legal document where I am saying we will not modify the misra rule texts and well I want to ensure nobody will complain later.

I think we need some generic infrastructure so addons can provide some optional extra attribute in output. I don't want to hardcode some specific handling for misra in Cppcheck.

@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Jun 23, 2019

maybe we can change the --cli output so it writes results in a json format instead. in that format we can have a some additional field for Mandatory/Recommended

@jubnzv
Copy link
Copy Markdown
Contributor Author

jubnzv commented Jun 23, 2019

@danmar, hm, really, it looks strange in GUI.

JSON, like any other intermediate representation, seems like a good idea. We could define set of formatting functions to generate human-readable reports for command line tools as good as save internal data in file and use it in GUI client, for example.

Right now I don't see a way to integrate additional addon-specific fields in existing UI. We need to separate addons output from cppcheck output. Do you have any suggestions?

@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Jun 24, 2019

I changed to JSON with c97dc79

There will be an "extra" attribute. Maybe we can use some other better name.

For now the CLI will not use that attribute but maybe that is fine for now. I guess the GUI needs to handle this somehow also. Could you update the misra addon so it outputs the "extra" attribute.

@jubnzv jubnzv deleted the misra-severity branch October 28, 2019 11:44
jubnzv added a commit to jubnzv/cppcheck that referenced this pull request Nov 13, 2019
* misra.py: Fixup rules severity

* Divide cppcheck and MISRA severity.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants