Commit graph

53 commits

Author SHA1 Message Date
MarcoFalke adf27b531a
Merge #13954: Warn (don't fail!) on spelling errors. Fix typos reported by codespell.
f8a81f73ac lint: Add spell check linter (codespell) (practicalswift)
ada356208e Fix typos reported by codespell (practicalswift)

Pull request description:

  * Check for common misspellings using `codespell`.
  * Fix recently introduced typos reported by `codespell`.

Tree-SHA512: 9974c0e640b411c7d0ebc5b45de253c19bac7fe3002cd98601ff8da8db584224c2fd7d331aee3df612c9f2cfef540d647a9b4c5a1a73fd208dc93ce4bf9e5e3e
2018-09-05 18:13:05 -04:00
MarcoFalke 97ccd2b84e
Merge #14115: lint: Make all linters work under the default macOS dev environment (build-osx.md)
341f7c7b0e macOS fix: Check for correct version of flake8 to avoid spurious warnings. The brew installed flake8 version is Python 2 based and does not work. (practicalswift)
908a559f33 macOS fix: Add excludes for checks added in the newer shellcheck version installed by brew (practicalswift)
ec4d57bbb3 macOS fix: Work around empty (sub)expression error when using BSD grep (practicalswift)
b57d7d92fe macOS fix: Avoid mapfile due to ancient version of bash shipped with macOS (practicalswift)

Pull request description:

  The linters are thoroughly tested under Ubuntu which is what we use in Travis. When reading #14041 I understood that some developers were experiencing problems when running the linters on their local machines.

  Assuming these local machines were running macOS I installed a fresh macOS VM, followed the instructions in `build-osx.md` and ran the linters.

  This PR contains the changes needed to make `lint-all.sh` run as expected.

  Ideally the linters would continuously run also under a Travis macOS environment to make sure we catch these kind of issues before merge.

Tree-SHA512: b39c9a970d14d27db1fb592539923c0bc676b5217f415d02fda3f17bf54d46faa172376e8a3ecab07ca68a3acba9aebe00b2b1b2161b2a36b85fbb672e7efb5c
2018-09-05 09:23:52 -04:00
practicalswift f8a81f73ac lint: Add spell check linter (codespell) 2018-09-04 13:11:26 +02:00
practicalswift ada356208e Fix typos reported by codespell 2018-09-04 13:11:26 +02:00
Wladimir J. van der Laan fa616275b2
Merge #14128: lint: Make sure we read the command line inputs using utf-8 decoding in python
5d62dcf9cf lint: Make sure we read the command line inputs using utf-8 decoding in python (Chun Kuan Lee)

Pull request description:

  Make sure we read the command line inputs using utf-8 decoding in python

  occurred from travis cron job:

  contrib/verify-commits/verify-commits.py should run with utf-8, otherwise it would raise UnicodeDecodeError
  `UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 744: ordinal not in range(128)`

Tree-SHA512: 90e4ad57fdbbbecb0a21fc2d2b03a04f5ef125e54124719ef36e5a85326930b732b47534757a7c3a8730096f3947b009ec898191928b5c2d38f9f4b3e37db48d
2018-09-04 12:34:43 +02:00
Chun Kuan Lee 5d62dcf9cf lint: Make sure we read the command line inputs using utf-8 decoding in python 2018-09-02 21:40:51 +08:00
Wladimir J. van der Laan 709a15b0a6
Merge #14088: tests: Don't assert(...) with side effects
ca1a093127 Add regression test: Don't assert(...) with side effects (practicalswift)
4c3c9c3869 Don't assert(...) with side effects (practicalswift)

Pull request description:

  Don't `assert(...)` with side effects.

  From the developer notes:

  > **Assertions should not have side-effects**
  >
  > Rationale: Even though the source code is set to refuse to compile with assertions disabled, having side-effects in assertions is unexpected and makes the code harder to understand

  These assertions were introduced quite recently (in #14069 which was merged two days ago) and since this is a recurring thing (see #13534 – "Don't assert(foo()) where foo() has side effects" from May) I added a simple regression test for the most obvious common side effect.

Tree-SHA512: be65db9d8d5d0f5752152ba73fe3fbb0531880f156d3cd7dfdf1752709979b63214e46ae64b1adbe1e09fa121278f4087f4ae49bff16cf8f5aec16ea6bde3650
2018-08-31 15:00:15 +02:00
practicalswift 341f7c7b0e macOS fix: Check for correct version of flake8 to avoid spurious warnings. The brew installed flake8 version is Python 2 based and does not work. 2018-08-31 00:58:47 +02:00
practicalswift 908a559f33 macOS fix: Add excludes for checks added in the newer shellcheck version installed by brew 2018-08-31 00:57:42 +02:00
practicalswift ec4d57bbb3 macOS fix: Work around empty (sub)expression error when using BSD grep 2018-08-31 00:52:48 +02:00
practicalswift b57d7d92fe macOS fix: Avoid mapfile due to ancient version of bash shipped with macOS 2018-08-31 00:51:43 +02:00
Wladimir J. van der Laan 5924dadc2f
Merge #13671: Remove the boost/algorithm/string/case_conv.hpp dependency
b193d5a443 Removes the Boost case_conv.hpp dependency. (251)
7a208d9fad Implements custom tolower and toupper functions. (251)
e2ba043b8d Implements ParseNetwork unit test. (251)

Pull request description:

  This pull request removes the `boost/algorithm/string/case_conv.hpp` dependency from the project.

  `boost/algorithm/string/case_conv.hpp` is included for the `boost::to_lower` and `boost::to_upper` template functions.

  We can replace the calls to these functions with straightforward alternative implementations that use the C++ Standard Library, because the functions are called with `std::string` objects that use standard 7-bit ASCII characters as argument.

  The refactored implementation should work without the explicit `static_cast<unsigned char>` cast and `unsigned char` lambda return type. Both have been added defensively and to be explicit. Especially in case of the former, behaviour is undefined (potentially result in a crash) if the `std::toupper` argument is not an `unsigned char`.

  A potential alternative, maybe even preferred, implementation to address the `boost::to_lower` function call in `ParseNetwork(std::string)` could have been:

  ```c++
  if (net == "ipv4" || net == "IPv4") return NET_IPV4;
  if (net == "ipv6" || net == "IPv6") return NET_IPV6;
  ```
  This alternative implementation would however change the external behaviour of `ParseNetwork(std::string)`.

  This pull requests includes a unit test to validate the implementation of `ParseNetwork(std::string)`  prior and after the removal of the `case_conv.hpp` dependency.

  `boost/algorithm/string/case_conv.hpp` has been removed from the `EXPECTED_BOOST_INCLUDES` in `test/lint/lint-includes.sh` because it is no longer required.

Tree-SHA512: d803ae709f2368a3efb223097384a722436955bce0c44a1a5cffd0abb3164be0cce85ba0e9ebd9408166df3f1a95ea0c0d29e3a2534af2fae206c0419d67fde9
2018-08-29 14:59:49 +02:00
251 b193d5a443 Removes the Boost case_conv.hpp dependency.
This commit removes the `boost/algorithm/string/case_conv.hpp` dependency from the project. It replaces the `boost::to_lower` and `boost::to_upper` functions with custom functions that are locale independent and ASCII deterministic.
2018-08-28 18:42:53 +02:00
practicalswift ca1a093127 Add regression test: Don't assert(...) with side effects 2018-08-28 14:28:14 +02:00
Chun Kuan Lee 1661a472b8 add unicode compatible file_lock for Windows
boost::interprocess::file_lock cannot open the files that contain characters which cannot be parsed by the user's code page on Windows.
This commit add a new class to handle those specific file for Windows.
2018-08-28 00:55:13 +08:00
Julian Fleischer 414326952c use export LC_ALL=C.UTF-8 2018-08-27 12:23:25 +02:00
Wladimir J. van der Laan f6eb85d17c
Merge #13707: tests: Add usage note to check-rpc-mappings.py
03a2d68010 Tests: add usage note to check-rpc-mappings.py (Mason Simon)

Pull request description:

  This test would previously fail without a user-friendly warning message, if invoked with no arguments.

  Test plan:
  ```
  bitcoin @_@$ python3 test/lint/check-rpc-mappings.py
  Usage: test/lint/check-rpc-mappings.py ROOT-DIR
  bitcoin @_@$ echo $?
  1
  bitcoin @_@$ python3 test/lint/check-rpc-mappings.py .
  * Checking consistency between dispatch tables and vRPCConvertParams
  bitcoin @_@$ echo $?
  0
  ```

Tree-SHA512: 0b7a94125d18ba5dbf3c3281a4af60718d2e495bf7645d7c2a3e1a2a1d80cffcea1383c90247186728037f1f2b38de75d0f59ccf7f792d69edd33884f3698c07
2018-08-25 22:41:33 +02:00
João Barbosa 3567b247f4 test: Add lint to prevent SIGNAL/SLOT connect style 2018-08-21 09:43:55 +01:00
MarcoFalke bffb35f876
Merge #13054: tests: Enable automatic detection of undefined names in Python tests scripts. Remove wildcard imports.
68400d8b96 tests: Use explicit imports (practicalswift)

Pull request description:

  Enable automatic detection of undefined names in Python tests scripts. Remove wildcard imports.

  Wildcard imports make it unclear which names are present in the namespace, confusing both readers and many automated tools.

  An additional benefit of not using wildcard imports in tests scripts is that readers of a test script then can infer the rough testing scope just by looking at the imports.

  Before this commit:

  ```
  $ contrib/devtools/lint-python.sh | head -10
  ./test/functional/feature_rbf.py:8:1: F403 'from test_framework.util import *' used; unable to detect undefined names
  ./test/functional/feature_rbf.py:9:1: F403 'from test_framework.script import *' used; unable to detect undefined names
  ./test/functional/feature_rbf.py:10:1: F403 'from test_framework.mininode import *' used; unable to detect undefined names
  ./test/functional/feature_rbf.py:15:12: F405 bytes_to_hex_str may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
  ./test/functional/feature_rbf.py:17:58: F405 CScript may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
  ./test/functional/feature_rbf.py:25:13: F405 COIN may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
  ./test/functional/feature_rbf.py:26:31: F405 satoshi_round may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
  ./test/functional/feature_rbf.py:26:60: F405 COIN may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
  ./test/functional/feature_rbf.py:30:41: F405 satoshi_round may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
  ./test/functional/feature_rbf.py:30:68: F405 COIN may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
  $
  ```

  After this commit:

  ```
  $ contrib/devtools/lint-python.sh | head -10
  $
  ```

Tree-SHA512: 3f826d39cffb6438388e5efcb20a9622ff8238247e882d68f7b38609877421b2a8e10e9229575f8eb6a8fa42dec4256986692e92922c86171f750a0e887438d9
2018-08-13 08:28:06 -04:00
practicalswift 68400d8b96 tests: Use explicit imports 2018-08-13 14:13:39 +02:00
practicalswift 4441ad677a Make format string linter understand basic template parameter syntax 2018-08-10 17:28:59 +02:00
Wladimir J. van der Laan 78dae8cacc
Merge #13780: 0.17: Pre-branch maintenance
3fc20632a3 qt: Set BLOCK_CHAIN_SIZE = 220 (DrahtBot)
2b6a2f4a28 Regenerate manpages (DrahtBot)
eb7daf4d60 Update copyright headers to 2018 (DrahtBot)

Pull request description:

  Some trivial maintenance to avoid having to do it again after the 0.17 branch off.

  (The scripts to do this are in `./contrib/`)

Tree-SHA512: 16b2af45e0351b1c691c5311d48025dc6828079e98c2aa2e600dc5910ee8aa01858ca6c356538150dc46fe14c8819ed8ec8e4ec9a0f682b9950dd41bc50518fa
2018-08-08 13:55:27 +02:00
practicalswift bcd4b0f5cd Add linting of WalletLogPrintf(...) format strings 2018-08-07 14:57:54 +02:00
practicalswift a3e4556949 build: Add format string linter
This linter checks that the number of arguments passed to each variadic format
string function matches the number of format specifiers in the format string.
2018-08-07 14:07:21 +02:00
Julian Fleischer 83c48d9a1f fix locale for lint-shell 2018-08-02 14:46:05 +02:00
DrahtBot eb7daf4d60 Update copyright headers to 2018 2018-07-27 07:15:02 -04:00
251 e3245f2e7b Removes Boost predicate.hpp dependency
This is a squashed commit that squashes the following commits:

This commit removes the `boost/algorithm/string/predicate.hpp` dependenc
from the project by replacing the function calls to `boost::algorithm::starts_with`
`boost::algorithm::ends_with` and `all` with respectively C++11'
`std::basic_string::front`, `std::basic_string::back`, `std::all_of` function calls

This commit replaces `boost::algorithm::is_digit` with  a locale independent isdigi
function, because the use of the standard library's `isdigit` and `std::isdigit
functions is discoraged in the developer notes
2018-07-22 21:34:45 +02:00
251 5f019d5354 Removes the boost/algorithm/string/join dependency
This commit removes the `boost/algorithm/string/join` dependency
from the project by replacing `boost::algorithm::join` with
a simple helper function.
2018-07-21 01:14:25 +02:00
Wladimir J. van der Laan 2dc5ab6378
Merge #13482: Remove boost::program_options dependency
f447a0a707 Remove program options from build system (Chun Kuan Lee)
11588c639e Replace boost program_options (Chun Kuan Lee)

Pull request description:

  Concept from #12744, but without parsing negated options.

Tree-SHA512: 7f418744bb8934e313d77a5f162633746ef5d043de802b9c9cd9f7c1842e7e566eb5f171cd9e2cc13317281b2449c6fbd553fa4f09b837e6af2f5d2b2aabdca2
2018-07-20 16:45:44 +02:00
Mason Simon 03a2d68010 Tests: add usage note to check-rpc-mappings.py 2018-07-19 11:26:21 -07:00
Ben Woosley 5c613aadd6
lint: Add linter for circular dependencies
Protects against added circular depencies, makes it explicit in the
code when circular dependencies have been removed.

Modeled after EXPECTED_BOOST_INCLUDES in lint-includes.sh
2018-07-18 09:11:26 -04:00
Chun Kuan Lee f447a0a707 Remove program options from build system 2018-07-18 02:48:34 +00:00
practicalswift 962d8eed5b Remove boost dependency (boost/assign/std/vector.hpp) 2018-06-27 17:45:18 +02:00
Wladimir J. van der Laan 9ab4c2a246
Merge #13496: Test: Harden lint-filenames.sh
927e1150bc Test: Harden lint-filenames.sh (wodry)

Pull request description:

  - This fixes that only files with lower case file name suffix where found before, which contradicted the Regex to find uppercase characters in file names (including suffixes I guess).
  - `--full-name` switch was added to git ls-files, to define that the found file always includes it's full path in the git project.
  - since we know now that the file name includes the full path, we can harden the Regex to exclude the  secp256k1 and univalue sub folders.
  - use backslash line break to make code easier to read and avoid too long line.

Tree-SHA512: 9b55fe4965ae2084112b9f8a81bf9c657756c2cb5004986e7b6102a76adaf62c7d7a53257d9f13c5d8a1c75870b52c744d13830e3edd454a099e810357c914e5
2018-06-24 18:39:27 +02:00
DesWurstes 000000035b Obsolete #!/bin/bash shebang 2018-06-20 11:12:41 +03:00
wodry 927e1150bc Test: Harden lint-filenames.sh 2018-06-18 18:02:11 +02:00
practicalswift 7b23e6e13f Follow-up to #13454: Fix broken build by exporting LC_ALL=C 2018-06-18 14:20:18 +02:00
Wladimir J. van der Laan 45c00f8416
Merge #13454: Make sure LC_ALL=C is set in all shell scripts
47776a958b Add linter: Make sure all shell scripts opt out of locale dependence using "export LC_ALL=C" (practicalswift)
3352da8da1 Add "export LC_ALL=C" to all shell scripts (practicalswift)

Pull request description:

  ~~Make sure `LC_ALL=C` is set when using `grep` range expressions.~~

  Make sure `LC_ALL=C` is set in all shell scripts.

  From the `grep(1)` documentation:

  > Within a bracket expression, a range expression consists of two characters separated by a hyphen. It matches any single character that sorts between the two characters, inclusive, using the locale's collating sequence and character set. For example, in the default C locale, `[a-d]` is equivalent to `[abcd]`. Many  locales sort characters in dictionary order, and in these locales `[a-d]` is typically not equivalent to `[abcd]`; it might be equivalent to `[aBbCcDd]`, for example. To obtain the traditional interpretation of bracket expressions, you can use the C locale by setting the `LC_ALL` environment variable to the value C.

  Context: [Locale issue found when reviewing #13450](https://github.com/bitcoin/bitcoin/pull/13450/files#r194877736)

Tree-SHA512: fd74d2612998f9b49ef9be24410e505d8c842716f84d085157fc7f9799d40e8a7b4969de783afcf99b7fae4f91bbb4559651f7dd6578a6a081a50bdea29f0909
2018-06-18 13:18:12 +02:00
Wladimir J. van der Laan a90ca4087a
Merge #13448: Add linter: Make sure we explicitly open all text files using UTF-8 encoding in Python
c8176b3cc7 Add linter: Make sure we explicitly open all text files using UTF-8 or ASCII encoding in Python (practicalswift)
634bd97001 Explicitly specify encoding when opening text files in Python code (practicalswift)

Pull request description:

  Add linter: Make sure we explicitly open all text files using UTF-8 encoding in Python.

  As requested by @laanwj in #13440.

Tree-SHA512: 1651c00fe220ceb273324abd6703aee504029b96c7ef0e3029145901762c733c9b9d24927da281394fd4681a5bff774336c04eed01fafea997bb32192c334c06
2018-06-16 15:23:14 +02:00
practicalswift ad691f666b Add linter: Enforce the source code file naming convention described in the developer notes 2018-06-14 22:40:41 +02:00
practicalswift 47776a958b Add linter: Make sure all shell scripts opt out of locale dependence using "export LC_ALL=C" 2018-06-14 15:27:52 +02:00
practicalswift 3352da8da1 Add "export LC_ALL=C" to all shell scripts 2018-06-14 15:27:52 +02:00
practicalswift c8176b3cc7 Add linter: Make sure we explicitly open all text files using UTF-8 or ASCII encoding in Python 2018-06-12 21:49:04 +02:00
practicalswift 634bd97001 Explicitly specify encoding when opening text files in Python code 2018-06-12 21:34:52 +02:00
Wladimir J. van der Laan ca2a23387b
Merge #13120: policy: Treat segwit as always active
fa7a6cf1b3 policy: Treat segwit as always active (MarcoFalke)

Pull request description:

  Now that segwit is active for a long time, there is no need to reject transactions with the reason that segwit hasn't activated.

  Strictly speaking, this is a bug fix, because with the release of 0.16, we create segwit transactions in our wallet by default without checking if they are allowed by local policy.

  More broadly, this simplifies the code as if "premature witness" was always set to true with the corresponding command line args.

Tree-SHA512: 484c26aa3a66faba6b41e8554a91a29bfc15fbf6caae3d5363a3966283143189c4bd5333a610b0669c1238f75620691264e73f6b9f1161cdacf7574d946436da
2018-06-12 17:20:34 +02:00
Wladimir J. van der Laan 7c32b414b6
Merge #13230: Simplify include analysis by enforcing the developer guide's include syntax
16e3cd380a Clarify include recommendation (practicalswift)
6d10f43738 Enforce the use of bracket syntax includes ("#include <foo.h>") (practicalswift)
906bee8e5f Use bracket syntax includes ("#include <foo.h>") (practicalswift)

Pull request description:

  When analysing includes in the project it is often assumed that the preferred bracket include syntax (`#include <foo.h>`) mentioned in `developer-docs.md` is used consistently. @sipa:s excellent circular dependencies script [`circular-dependencies.py`](50c69b7801/contrib/devtools/circular-dependencies.py) (#13228) is an example of a script making this reasonable assumption.

  This PR enables automatic Travis checking of the include syntax making sure that the bracket syntax includes (`#include <foo.h>`) is used consistently.

Tree-SHA512: a414921aabe8e487ebed42f3f1cbd02fecd1add385065c1f2244cd602c31889e61fea5a801507ec501ef9bd309b05d3c999f915cec1c2b44f085bb0d2835c182
2018-06-11 20:24:58 +02:00
Wladimir J. van der Laan 5779dc4f76
Merge #13041: build: Add linter checking for accidental introduction of locale dependence
698cfd0811 docs: Mention lint-locale-dependence.sh in developer-notes.md (practicalswift)
0a4ea2f458 build: Add linter for checking accidental locale dependence (practicalswift)

Pull request description:

  This linter will check for code accidentally introducing locale dependencies.

  Unnecessary locale dependence can cause bugs that are very tricky to isolate and fix. We should avoid using locale dependent functions if possible.

  Context: https://github.com/bitcoin/bitcoin/pull/12881#issuecomment-378564722

  Example output:

  ```
  $ contrib/devtools/lint-locale-dependence.sh
  The locale dependent function tolower(...) appears to be used:
  src/init.cpp:    if (s[0] == '0' && std::tolower(s[1]) == 'x') {

  Unnecessary locale dependence can cause bugs that are very
  tricky to isolate and fix. Please avoid using locale dependent
  functions if possible.

  Advice not applicable in this specific case? Add an exception
  by updating the ignore list in contrib/devtools/lint-locale-dependence.sh
  ```

  **Note to reviewers:** What is the most appropriate `LOCALE_DEPENDENT_FUNCTIONS` function list? What should be added or removed?

Tree-SHA512: 14e448828804bb02bf59070647e38b52fce120c700c903a4a8472769a2cee5dd529bd3fc182386993cb8720482cf4250b63a0a477db61b941ae4babe5c65025f
2018-06-07 08:56:46 +02:00
Ben Woosley 9d6c9dbb88
lint: Add linter to error on #include <*.cpp>
Files should depend on one another by interface, not by implementation.
This checks for quoted includes as well.

With practicalswift
2018-06-06 02:11:25 -07:00
practicalswift 6d10f43738 Enforce the use of bracket syntax includes ("#include <foo.h>") 2018-06-06 11:09:05 +02:00
practicalswift 0a4ea2f458 build: Add linter for checking accidental locale dependence 2018-06-06 08:08:44 +02:00