MAINT: Cleanup pandas/src/parser
State: closed

Remove dead code and reformat for style using Google's C++ style guide (found <a href="https://google.github.io/styleguide/cppguide.html">here</a>).

Also adds line of code to install fork of Google's C++ style checker (source code <a href="https://github.com/cpplint/cpplint">here</a>) when doing style checks in Travis.

Labels:
name: IO CSV
description: read_csv, to_csv
name: Clean

Comments:
## [Current coverage](https://codecov.io/gh/pandas-dev/pandas/pull/14740?src=pr) is 85.28% (diff: 100%)
> Merging [#14740](https://codecov.io/gh/pandas-dev/pandas/pull/14740?src=pr) into [master](https://codecov.io/gh/pandas-dev/pandas/branch/master?src=pr) will increase coverage by **<.01%**

```diff
@@             master     #14740   diff @@
==========================================
  Files           144        144          
  Lines         50953      50968    +15   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43452      43467    +15   
  Misses         7501       7501          
  Partials          0          0          
```

> Powered by [Codecov](https://codecov.io?src=pr). Last update [4378f82...b2c4211](https://codecov.io/gh/pandas-dev/pandas/compare/4378f82967f59097055eef17ede50aa515525551...b2c4211b10c3da552bb832eb56ede0cc0356d575?src=pr)
@jreback : All green, so ready to merge if there are no other concerns.
@gfyoung can you add the checker in, you can even do it in another PR so we can see the failures.
@jreback : Here's the command I ran (after doing `pip install cpplint`, source code <a href="https://github.com/cpplint/cpplint">here</a>):
~~~
cpplint --extensions=c --headers=h /home/User/Repositories/pandas/pandas/src/parser/tokenizer.c
~~~

Note: you can disable extensions: e.g. `--filter=-readability/casting,-runtime/int`.  If you want to pipe to a file instead of everything on the terminal, just append `&> <filename>`

Here's what the linter is complaining about right now:
~~~
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:20:  Include the directory when naming .h files  [build/include_subdir] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:48:  Using C-style cast.  Use reinterpret_cast<coliter_t *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:133:  Using C-style cast.  Use reinterpret_cast<parser_t *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:136:  Using C-style cast.  Use reinterpret_cast<void *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:137:  Using C-style cast.  Use reinterpret_cast<void *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:138:  Using C-style cast.  Use reinterpret_cast<void *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:139:  Using C-style cast.  Use reinterpret_cast<void *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:140:  Using C-style cast.  Use reinterpret_cast<void *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:148:  Using C-style cast.  Use reinterpret_cast<void *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:149:  Using C-style cast.  Use reinterpret_cast<void *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:152:  Using C-style cast.  Use reinterpret_cast<kh_int64_t *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:185:  Using C-style cast.  Use reinterpret_cast<char *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:196:  Using C-style cast.  Use reinterpret_cast<char **>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:197:  Using C-style cast.  Use reinterpret_cast<int *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:202:  Using C-style cast.  Use reinterpret_cast<int *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:204:  Using C-style cast.  Use reinterpret_cast<int *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:254:  Using C-style cast.  Use reinterpret_cast<void *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:258:  Using C-style cast.  Use reinterpret_cast<char *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:285:  Using C-style cast.  Use reinterpret_cast<char **>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:301:  Using C-style cast.  Use reinterpret_cast<void *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:306:  Using C-style cast.  Use reinterpret_cast<int *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:315:  Using C-style cast.  Use reinterpret_cast<int *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:328:  Using C-style cast.  Use reinterpret_cast<void *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:333:  Using C-style cast.  Use reinterpret_cast<int *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:348:  Using C-style cast.  Use reinterpret_cast<char *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:364:  Using C-style cast.  Use reinterpret_cast<char *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:400:  Using C-style cast.  Use reinterpret_cast<char *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:401:  Almost always, snprintf is better than strcpy  [runtime/printf] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:406:  Using C-style cast.  Use reinterpret_cast<char *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:407:  Almost always, snprintf is better than strcpy  [runtime/printf] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:458:  Using C-style cast.  Use reinterpret_cast<char *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:470:  Using C-style cast.  Use reinterpret_cast<char *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:471:  Never use sprintf. Use snprintf instead.  [runtime/printf] [5]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:501:  Using C-style cast.  Use reinterpret_cast<char *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:502:  Never use sprintf. Use snprintf instead.  [runtime/printf] [5]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:528:  Using C-style cast.  Use reinterpret_cast<void *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:531:  Using C-style cast.  Use reinterpret_cast<kh_int64_t *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:561:  Using C-style cast.  Use reinterpret_cast<char *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:564:  Never use sprintf. Use snprintf instead.  [runtime/printf] [5]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:567:  Never use sprintf. Use snprintf instead.  [runtime/printf] [5]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:590:  Using C-style cast.  Use reinterpret_cast<char *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:591:  Never use sprintf. Use snprintf instead.  [runtime/printf] [5]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:675:  Using C-style cast.  Use reinterpret_cast<kh_int64_t *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:676:  Using C-style cast.  Use reinterpret_cast<kh_int64_t *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:684:  Use int16/int64/etc, rather than the C type long  [runtime/int] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:991:  Using C-style cast.  Use reinterpret_cast<char *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:992:  Never use sprintf. Use snprintf instead.  [runtime/printf] [5]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1101:  Using C-style cast.  Use reinterpret_cast<char *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1107:  Using C-style cast.  Use reinterpret_cast<char *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1108:  Never use sprintf. Use snprintf instead.  [runtime/printf] [5]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1147:  Using C-style cast.  Use reinterpret_cast<void *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1196:  Using C-style cast.  Use reinterpret_cast<void *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1200:  Using C-style cast.  Use reinterpret_cast<char **>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1202:  Using C-style cast.  Use reinterpret_cast<void *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1206:  Using C-style cast.  Use reinterpret_cast<int *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1221:  Using C-style cast.  Use reinterpret_cast<void *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1231:  Using C-style cast.  Use reinterpret_cast<char *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1234:  Using C-style cast.  Use reinterpret_cast<char *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1247:  Using C-style cast.  Use reinterpret_cast<void *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1251:  Using C-style cast.  Use reinterpret_cast<int *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1253:  Using C-style cast.  Use reinterpret_cast<void *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1257:  Using C-style cast.  Use reinterpret_cast<int *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1295:  Using C-style cast.  Use static_cast<int>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1348:  Use int16/int64/etc, rather than the C type long  [runtime/int] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1371:  Almost always, snprintf is better than strcpy  [runtime/printf] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1399:  Use int16/int64/etc, rather than the C type long  [runtime/int] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1407:  Using C-style cast.  Use static_cast<int>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1465:  Using C-style cast.  Use reinterpret_cast<char *>(...) instead  [readability/casting] [4]
/home/User/Repositories/pandas/pandas/src/parser/tokenizer.c:1589:  Using C-style cast.  Use reinterpret_cast<char *>(...) instead  [readability/casting] [4]
Done processing /home/User/Repositories/pandas/pandas/src/parser/tokenizer.c
Total errors found: 69
~~~
Immediate thoughts:

* C-style casting concern is not related to us, so we can disable `readability/casting`
* int16/int64 not really problematic I think because it's C, so I think we can also disable `runtime/int`

* A little confused why the `build/include_subdir` is a problem since `tokenizer.h` is in the same dir

* The `snprintf` vs `sprintf` is actually valid one IMO because it protects against buffer overflow, and I think we actually have been violating this one pretty frequently.  For example, when I limit the error message to `sizeof(self->error_msg)`, ALL of the error messages get truncated.

Related SO <a href="http://stackoverflow.com/questions/7315936/which-of-sprintf-snprintf-is-more-secure">here</a>

* Similar opinion about `snprintf` vs. `strcpy`
@jreback : Any thoughts / updates on this?
i would like to include a checker for the syntax
@jreback : That doesn't address the question about the syntax errors I'm getting from it though.
not sure what you mean

if this is changed gen we need a checker to assure it is not changed just for style 
Right, but we need to figure which things to actually respect.  See the errors that I get out <a href="https://github.com/pandas-dev/pandas/pull/14740#issuecomment-263397544">above</a>.  We can't just throw a style checker in without knowing what we're checking, especially since this is geared towards C++ and not C in particular.
as I said above, let's find a workable checker then. its actually only important this actually runs on linux/osx as that is where it is checked (of course if it works on windows that is great but not necessary).

We can start with an optional check and make it strict as needed. 
@jreback : So you mean just include the checker but don't fail if the style check fails right now?
well I would like to fix this, but only if we have a checker
@jreback : Well, we do have a checker, but before we "fix," need to figure which "errors" are actually valid for our purposes (see <a href="https://github.com/pandas-dev/pandas/pull/14740#issuecomment-263399005">above</a>).
@gfyoung sure. so put together a minimal set of checks and change the code to match those. over time we can become more strict by adding options (or requiring more checks) in the linter. 


@jreback : Fair enough.  Will start with the `io/parser` files and can then expand later
@jreback : Successfully incorporated linter on Travis.  Ready to merge if there are no other concerns.
lgtm. ping on green.
@jreback : Everybody is green.  Ready to merge if there are no other concerns.

Also, as a follow-up, should all of these files <a href="https://github.com/pandas-dev/pandas/search?utf8=%E2%9C%93&q=language%3Ac&type=Code">here</a> be fair game for the linter in the future?
@gfyoung I would do those additional files, but exclude src/headers & src/klib

IOW include src/datetime
@jreback : Sounds good.  I'll do that as a follow-up then after this is merged.
thanks @gfyoung 

@jorisvandenbossche you *may* need to pick this for 0.19.2 (if we have fixes that depend on it).
So from the list in <a href="https://github.com/pandas-dev/pandas/tree/cebc70cc277462176edea73bb71eaad9f83e9f47/pandas/src">pandas/src</a>:
* **datetime** : **YES**
* **headers** : **NO**
* **klib** : **NO**
* **msgpack** : **??** (leaning no)
* **parser** : **YES**
* **ujson** : **??** (leaning no?)
* **period_helper.c** : **??** (leaning no since borrowed)

@jreback : Can you confirm the other three?
msgpack -> no
ujson -> yes (its local code, originally came from ujson though)

not sure where period_helper.c came from (but prob ok to clean)

@jreback yes, it will depend on whether there will still be fixes to the parser for 0.19.2 and how complex they are. But if possible, would like to keep this for 0.20.0 as it's purely clean-up and not needed for 0.19.2
@jreback : `period_helper.c` came from `scikit`, but as I am linting `datetime` (derived from `NumPy`), I suppose why not.