CPPCheckBear.py: Support language parameter #2710

gutsytechster · 2018-10-12T17:31:31Z

It adds the command line argument 'language' which can be
given whenever using the CPPCheckBear. It can hold two values
either C or C++.

Closes #2450

For short term contributors: we understand that getting your commits well
defined like we require is a hard task and takes some learning. If you
look to help without wanting to contribute long term there's no need
for you to learn this. Just drop us a message and we'll take care of brushing
up your stuff for merge!

Checklist

I read the commit guidelines and I've followed
them.
I ran coala over my code locally. (All commits have to pass
individually. It is not sufficient to have "fixup commits" on your PR,
our bot will still report the issues for the previous commit.) You will
likely receive a lot of bot comments and build failures if coala does not
pass on every single commit!

After you submit your pull request, DO NOT click the 'Update Branch' button.
When asked for a rebase, consult coala.io/rebase
instead.

Please consider helping us by reviewing other peoples pull requests as well:

pick up any PR at https://coala.io/review
review it (check https://coala.io/reviewing for more info)
if you are sure that it needs work, use corobo mark wip <URL> to get it out
of the review queue.

The more you review, the more your score will grow at coala.io and we will
review your PRs faster!

jayvdb · 2018-10-13T01:42:24Z

bears/c_languages/CPPCheckBear.py

        """
        args = ('--template={line}:{severity}:{id}:{message}',)
        files = tuple(self.file_dict.keys())

        if enable:
            args += ('--enable=' + ','.join(enable),)

+        if language:
+            args += ('--language={language}',)


CI is failing because you need to add tests for these lines.

@jayvdb , can you guide me a bit what should language parameter would do if we provide it.
Because in tests, already defined parameters are giving error or warnings.
But I am unable to understand what would it return.

If existings tests are failing on your PR, it is because your code is buggy.

The existing tests for CPPCheckBear work correctly in the CI on master builds and other peoples PRs.

gutsytechster · 2018-11-12T15:59:46Z

I have two different versions of cppcheck installed on two different machines. One of them has version 1.76.1 and other one has version 1.82 installed. However, when I used cppcheck on the newly added test file language_test.h which is a header file containing a bit of c++ specific code, on version 1.82, it did give an error like Code 'classFoo{' is invalid C code. Use --std or --language to configure the language. which means that version 1.82 considering .h files to be C specific. However, on cppcheck version 1.76.1 it doesn't give any error.
It seems that coala tests are using the previous version of cppcheck, and that's the reason the tests are failing.
What should be done here?

jayvdb · 2018-11-13T00:22:54Z

@gutsytechster , you need to build a test case which works with the version of cppclean that coala uses.
See the file bear-requirements.txt to see which version that is.

gutsytechster · 2018-11-13T07:25:25Z

Isn't cppcheck and cppclean different?
And doesn't CPPCheckBear uses cppcheck?

jayvdb · 2018-11-13T08:14:01Z

Yes, my apology, I should have said cppcheck

jayvdb

needs rebase and tests are failing

gutsytechster · 2019-01-14T17:20:57Z

@jayvdb can you review it?

I just need to know if I am taking the correct approach for using the Language class defined.

rajgoesout

You are getting errors in the tests. After changing the language parameter, try to go through the logs and rectify your tests.

rajgoesout · 2019-01-19T15:48:23Z

bears/c_languages/CPPCheckBear.py

@@ -30,18 +33,37 @@ class CPPCheckBear:

    def create_arguments(self, config_file,
                         enable: typed_list(str) = [],
+                         language: str = '',


Use this:

Suggested change

language: str = '',

language: Language = Language['Unknown'],

It will set a default of None.

Thanks @rajdeepbharati for your review!

The part where I am confused is that the whichever parameter the user will input would be treated as a string so how come the value of variable language will be of Language type.

So, if I input --language=c++, it will be treated as "c++".
But what I need to do is that it should be like Language['c++'].

Hello @gutsytechster do you still plan to work on this ? We need it for our game (play0ad.com)

You can't do what you want in one line. This won't work :

def create_arguments(self, config_file, enable: typed_list(str) = [], language: str = Language[language]):

From what I understand the default should be language = None

def create_arguments(self, config_file, enable: typed_list(str) = [], language: str = None):

Then line 55 check for

if language is not None: try: lang = Language[language] if isinstance(lang, Language.CPP): args += ('--language=c++',) elif isinstance(lang, Language.C): args += ('--language=c',) else: logging.error('Language can be either c or c++') return except(AttributeError): logging.error('Language can be either c or c++') return else args += ('--language=c',)

@StanleySweet, thanks for update on this.
Yes, I'll finish the issue :)

Cool. I think you need to add tests now. The Appveyor fails because the coberage went from 100% to 87 something
Not sure why travis fails but I guess it's related
Do younow how to write the tests ?

Oh yeah! I am trying to resolve these issues

KVGarg · 2019-08-21T17:54:43Z

tests/c_languages/CPPCheckBearTest.py

        self.section.append(Setting('enable', 'unusedFunction'))
+        self.section.append(Setting('language', 'c++'))


Missing self.section.language = Language[<language_name>]
Please check other bears tests to get more information on this. As well suggesting you to have a look what does run_section_from_bear does! You can find this function definition in coala repository.

@KVGarg, do we add a Language type parameter using self.section.language = Language[<language_name>] instead of self.section.append(Setting('language', 'c++')) or alongwith self.section.append(Setting('language', 'c++'))?

along with the line you’ve already added to the new tests!

KVGarg · 2019-08-21T17:55:27Z

tests/c_languages/CPPCheckBearTest.py


-    def test_no_enable_entered(self):
+    def test_language_c(self):
+        self.section.append(Setting('language', 'c'))
        results = self.get_results(self.test_files)
        messages = [result.message for result in results]
        self.assertEqual(len(messages), 1)


Test were reporting a different number. Please check that!

This test is getting passed on my local machine. I am not sure what else it is expecting?

May the messages count is greater than 3, due to above reason. Just check with that first.
btw i reported this change request because https://ci.appveyor.com/project/coala/coala-bears/builds/26862272/job/81isokrqkuiupbf1#L1446 this was logging that there are 3 messages

jayvdb · 2019-08-22T04:51:49Z

Tests are still missing 100% coverage

gutsytechster · 2019-08-22T07:20:10Z

Hey @jayvdb can you help to find why appveyor and travis are giving different assertion errors and even though they are passing on local?

It adds the command line argument 'language' which can be given whenever using the CPPCheckBear. It can hold two values either c or c++. Closes coala#2450

gutsytechster · 2019-08-22T08:37:53Z

On researching, I found that the Travis is failing because it is using cppcheck 1.52(https://travis-ci.org/coala/coala-bears/jobs/575212105#L295) which doesn't support the language parameter and hence generating error resulting in return a None object instead of a list type object.

gutsytechster · 2019-08-22T08:48:17Z

Appveyor is failing because it is using cppcheck 1.88(https://ci.appveyor.com/project/coala/coala-bears/builds/26877468/job/i2incdndjdui7jvf#L246) which generates different output when using the language parameter. As expected, it does generate 3 error when I specify --language=c though not for the older versions.

As far as I've understood, the cause for failing tests is because of using different cppcheck version across different test environments. Since different version acts differently, the tests are failing.
@jayvdb kindly have a look.

Shouldn't we use the same versions for apt packages across all testing environments?

StanleySweet · 2019-09-09T14:20:35Z

Any news ?

gutsytechster · 2019-09-10T07:02:28Z

@StanleySweet the parameter seems to work. But as I explained above, the tests are failing due to different versions of cppcheck for Appveyor and Travis. IMO the solution to it would be to use the same cppcheck version which also supports the use of language parameter.
I am waiting for the reviews over it.

StanleySweet · 2019-09-10T08:21:31Z

Yeah, I was actually just bumping the ticket since it had been 18 days.

graugans · 2020-02-19T19:19:55Z

Hey any news on this? For a C++17 code base a cppcheck version 1.90 is needed what the coala.io does not support due to the fact this PR is not merged.... Should this PR also address the change in the CI scripts? .travis.yml

StanleySweet · 2020-02-19T19:32:24Z

I'm also interested.

StanleySweet · 2020-07-05T22:19:14Z

@jayvdb Anything we can do to get this merged?

StanleySweet · 2020-08-02T15:16:01Z

@abhishalya Any news?

gitmate-bot added process/pending review size/XS area/lintbears difficulty/low labels Oct 12, 2018

gitmate-bot assigned gutsytechster Oct 12, 2018

jayvdb requested changes Oct 13, 2018

View reviewed changes

jayvdb added process/wip and removed process/pending review labels Oct 13, 2018

gutsytechster force-pushed the issue2450 branch from fa4bf9b to a9c5350 Compare November 12, 2018 13:24

gitmate-bot added process/pending review and removed process/wip labels Nov 12, 2018

gutsytechster force-pushed the issue2450 branch from a9c5350 to 170ed5e Compare November 12, 2018 14:08

jayvdb added process/wip and removed process/pending review labels Nov 13, 2018

gutsytechster force-pushed the issue2450 branch from 170ed5e to cb88880 Compare November 13, 2018 08:47

gitmate-bot added process/pending review and removed process/wip labels Nov 13, 2018

gutsytechster force-pushed the issue2450 branch from cb88880 to 85bf532 Compare November 13, 2018 08:49

jayvdb requested changes Nov 13, 2018

View reviewed changes

jayvdb added process/wip and removed process/pending review labels Nov 13, 2018

gutsytechster force-pushed the issue2450 branch from 85bf532 to 00d7c03 Compare November 15, 2018 16:57

gitmate-bot added process/pending review and removed process/wip labels Nov 15, 2018

gutsytechster force-pushed the issue2450 branch 2 times, most recently from c2f98b9 to cdfb5bb Compare November 15, 2018 18:35

jayvdb removed the process/pending review label Dec 1, 2018

gutsytechster force-pushed the issue2450 branch from 2c10e0d to a997f62 Compare December 22, 2018 14:37

gitmate-bot added process/pending review and removed process/wip labels Dec 22, 2018

rajgoesout reviewed Jan 19, 2019

View reviewed changes

jayvdb added process/wip and removed process/pending review labels Feb 4, 2019

gutsytechster force-pushed the issue2450 branch 4 times, most recently from 63ce567 to fdfe0d1 Compare August 21, 2019 17:40

KVGarg suggested changes Aug 21, 2019

View reviewed changes

gutsytechster force-pushed the issue2450 branch from fdfe0d1 to c4e3c6f Compare August 21, 2019 18:31

gutsytechster force-pushed the issue2450 branch 2 times, most recently from 00fa538 to 4b4aa73 Compare August 22, 2019 06:30

CPPCheckBear.py: Support language parameter

cd36984

It adds the command line argument 'language' which can be given whenever using the CPPCheckBear. It can hold two values either c or c++. Closes coala#2450

gutsytechster force-pushed the issue2450 branch from 4b4aa73 to cd36984 Compare August 22, 2019 07:26

gutsytechster requested a review from jayvdb September 10, 2019 07:10

	language: str = '',
	language: Language = Language['Unknown'],

		self.section.append(Setting('enable', 'unusedFunction'))
		self.section.append(Setting('language', 'c++'))

CPPCheckBear.py: Support language parameter #2710

Are you sure you want to change the base?

CPPCheckBear.py: Support language parameter #2710

Conversation

gutsytechster commented Oct 12, 2018 • edited

Checklist

Choose a reason for hiding this comment

gutsytechster Oct 13, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gutsytechster commented Nov 12, 2018 • edited

jayvdb commented Nov 13, 2018

gutsytechster commented Nov 13, 2018

jayvdb commented Nov 13, 2018

jayvdb left a comment

Choose a reason for hiding this comment

gutsytechster commented Jan 14, 2019 • edited

rajgoesout left a comment • edited

Choose a reason for hiding this comment

rajgoesout Jan 19, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StanleySweet Aug 20, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jayvdb commented Aug 22, 2019

gutsytechster commented Aug 22, 2019

gutsytechster commented Aug 22, 2019

gutsytechster commented Aug 22, 2019 • edited

StanleySweet commented Sep 9, 2019

gutsytechster commented Sep 10, 2019

StanleySweet commented Sep 10, 2019

graugans commented Feb 19, 2020 • edited

StanleySweet commented Feb 19, 2020

StanleySweet commented Jul 5, 2020

StanleySweet commented Aug 2, 2020

gutsytechster commented Oct 12, 2018 •

edited

gutsytechster Oct 13, 2018 •

edited

gutsytechster commented Nov 12, 2018 •

edited

gutsytechster commented Jan 14, 2019 •

edited

rajgoesout left a comment •

edited

rajgoesout Jan 19, 2019 •

edited

StanleySweet Aug 20, 2019 •

edited

gutsytechster commented Aug 22, 2019 •

edited

graugans commented Feb 19, 2020 •

edited