← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/codecheck into lp:widelands

 

> Nice intiative :)

Thanks, but this also scratches two concrete itches I had: 
1) I added a new lint in another branch and found out that you cannot reliably run the lint tests anymore because some fail.
2) Errors in the docs trigger an email per hour that ends up in my inbox. There are currently errors in the docs and I'd like the email flood to stop.

cppcheck is orthogonal to these things - but I agree that it would be useful.

And yes, errors in the docs and in the lint testsuite will now break the build.

Diff comments:

> === modified file '.travis.sh'
> --- .travis.sh	2017-06-27 16:45:47 +0000
> +++ .travis.sh	2017-07-03 13:52:43 +0000
> @@ -42,8 +43,22 @@
>  cd build
>  cmake .. -DCMAKE_BUILD_TYPE:STRING="$BUILD_TYPE"
>  
> -# Any codecheck warning is an error in Debug builds. Keep the codebase clean!!
>  if [ "$BUILD_TYPE" == "Debug" ]; then
> +
> +   # Build the documentation. Any warning is an error.
> +   sudo pip install sphinx

yes (math), and trusty is ancient.

> +   pushd ../doc/sphinx
> +   mkdir source/_static
> +   ./extract_rst.py
> +   sphinx-build -W -b json -d build/doctrees source build/json
> +   popd
> +
> +   # Run the codecheck test suite.
> +   pushd ../cmake/codecheck
> +   ./run_tests.py
> +   popd
> +
> +   # Any codecheck warning is an error in Debug builds. Keep the codebase clean!!

Any new warnings will break the build. I checked that this is the case.

>     # Suppress color output.
>     TERM=dumb make -j1 codecheck 2>&1 | tee codecheck.out
>     if grep '^[/_.a-zA-Z]\+:[0-9]\+:' codecheck.out; then 
> 
> === modified file 'cmake/codecheck/rules/camel_case_for_classes'
> --- cmake/codecheck/rules/camel_case_for_classes	2014-09-18 15:53:08 +0000
> +++ cmake/codecheck/rules/camel_case_for_classes	2017-07-03 13:52:43 +0000
> @@ -13,30 +13,43 @@
>        if line.count("in_port_t = uint16_t") or line.count("in_addr_t = uint32_t"):
>           continue
>  
> -      if re.match("(^\s*((class|struct)\s+((\S+_)|([a-z]\S+)).*{)|((enum)\s+((\S+_)|(([abd-z])|(c(?!lass))\S+)).*{)|((using)\s+((\S+_)|([a-z]\S+)).*=).*$)", line):
> -
> -         errors.append((fn, lineno+1, "Use CapitalLetterWithCamelCase when naming an enum class, struct, or \"using\"."))
> +      words = line.strip().split()
> +      if (len(words) < 2 or words[0] not in ["class", "struct", "enum", "using"]
> +              or words[1] == "{"):
> +         continue
> +      if words[0] == "using" and words[1] == "namespace" or words[1].startswith('std::'):
> +         continue
> +      if words[0] == "enum":
> +         if words[1] == "class":
> +            words.pop(1)
> +            if len(words) < 2:
> +               continue
> +      if words[0] in ["class", "struct", "enum"] and words[-1] != "{":
> +         continue
> +      if re.match(r"([A-Z][a-z0-9]*)+", words[1]):
> +         continue
> +      errors.append((fn, lineno+1, "Use CapitalLetterWithCamelCase when naming an enum class, struct, or \"using\"."))
>  
>     return errors
>  # /end evaluate_matches
>  
>  forbidden = [

Yes. I linked the bug.

> -    "class my_class",
> -    "class myClass",
> -    "struct my_struct",
> -    "struct myStruct",
> -    "using my_typedef",
> -    "using myTypedef",
> -    "enum class my_enum",
> -    "enum class myEnum"
> -    "enum my_enum",
> -    "enum myEnum"
> +    "class my_class {",
> +    "class myClass {",
> +    "struct my_struct {",
> +    "struct myStruct {",
> +    "using my_typedef {",
> +    "using myTypedef {",
> +    "enum class my_enum {",
> +    "enum class myEnum {"
> +    "enum my_enum {",
> +    "enum myEnum {"
>  ]
>  
>  allowed = [
> -    "class MyClass",
> -    "struct MyStruct",
> -    "using MyTypedef",
> -    "enum class MyEnum",
> -    "enum MyEnum"
> +    "class MyClass {",
> +    "struct MyStruct {",
> +    "using MyTypedef {",
> +    "enum class MyEnum {",
> +    "enum MyEnum {"
>  ]


-- 
https://code.launchpad.net/~widelands-dev/widelands/codecheck/+merge/326647
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/codecheck into lp:widelands.


References