consolidate duplicate conditions in TextReader

[jameslamb]

I started exploring cppcheck, a static analyzer for C++ projects, tonight.

This PR proposes fixing one of the warnings raised by that tool. It caught some cases where two consecutive code blocks were guarded by identical if() conditions. Consolidating those blocks removes two unnecessary checks.

include/LightGBM/utils/text_reader.h:202:11: style: The if condition is the same as the previous if condition [duplicateCondition]
      if (is_used) { lines_.emplace_back(buffer, size); }
          ^
include/LightGBM/utils/text_reader.h:201:11: note: First condition
      if (is_used) { out_used_data_indices->push_back(line_idx); }
          ^
include/LightGBM/utils/text_reader.h:202:11: note: Second condition
      if (is_used) { lines_.emplace_back(buffer, size); }
          ^
include/LightGBM/utils/text_reader.h:217:11: style: The if condition is the same as the previous if condition [duplicateCondition]
      if (is_used) {
          ^
include/LightGBM/utils/text_reader.h:216:11: note: First condition
      if (is_used) { out_used_data_indices->push_back(line_idx); }
          ^
include/LightGBM/utils/text_reader.h:217:11: note: Second condition
      if (is_used) {
          ^

how I ran 'cppcheck' (click me)

On my Mac, I installed it with homebrew.

brew install cpplint

Then ran the command from the root of the repo, with the following command.

cppcheck \
    --force \
    --enable=all \
    --std=c11 \
    -I include/ \
    R-package/src \
> cppcheck.txt 2>&1

• --force says "test all combinations of #ifdefs" (by default, cppcheck will only check 12 combinations).
• --enable=all says "show all types of warnings"

NOTE: I only checked R-package/src in this example because I wanted to start by checking that relatively limited piece of LightGBM. Checking everything inn src/ (from the package root) returns a ton of notes. More opportunities for improvements in the future!

[StrikerRUS]

LGTM!
Will it make sense to run cppcheck at our CI on a regular basis like we run cpplint?

LightGBM/.ci/test.sh

Lines 82 to 83 in a7ff117

	echo "Linting C++ code"
	cpplint --filter=-build/c++11,-build/include_subdir,-build/header_guard,-whitespace/line_length --recursive ./src ./include ./R-package ./swig ./tests || exit -1

[jameslamb]

Will it make sense to run cppcheck at our CI on a regular basis like we run cpplint?

Maybe! I'm just getting familiar with it, so not sure yet how useful it is. Seems promising so far!

I'll open an issue for it like we did with cpplint (#1990) and mypy (#3867), including the full logs. And we can try running it at CI with || true for a while just to see the logs.

[jameslamb]

I've opened #4539 documenting how we might run cppcheck. I'll upload logs from a local run on my laptop there when it finishes running (might take about 10 minutes).

[StrikerRUS]

Sounds good, thanks!

[github-actions]

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.