Backport useful lints from upstream #4829

str4d · 2020-10-27T12:37:41Z

Cherry-picked from the following upstream PRs:

Several of the lints fail for our current codebase; these will be addressed in a
subsequent PR.

str4d · 2020-10-27T12:42:08Z

Once I've checked for any further relevant backports, I'll apply the following patch (so that GitHub doesn't get confused about commit ordering) EDIT: Done.

str4d · 2020-10-27T15:39:54Z

I'll rebase this once #4831 is merged (which conflicts with this).

zkbot · 2020-10-27T18:03:05Z

☔ The latest upstream changes (presumably #4831) made this pull request unmergeable. Please resolve the merge conflicts.

- ensure header namespaces and end comments are correct - add missing header end comments - ensure minimal formatting (add newlines etc.)

Identifiers beginning with an underscore followed immediately by an uppercase letter are reserved.

This adds a new CHECK_DOC check that looks for newly introduced trailing whitespace. Existing trailing whitespace (of which there is plenty!) will not trigger an error. This is written in a generic way so that new lint-*.sh scripts can be added to contrib/devtools/, as I'd like to contribute additional lint checks in the future.

Lint checks should not test these, they are historical documents, and we don't want to encourage silly changes to them to satisfy a checker. Hopefully makes travis pass again on master. Tree-SHA512: 37e6716c4fd5e8a4e579f9b84042e6b0ac224836b6c851cd1ca3f7d46611ffd3003bed0ae08dd0457f69d6eaa485a0d21c631e7ef16b14bdb0f2f78ea700332d

MacOS does not support 'grep -P' out of the box. This change makes it easier for developers to check for whitespace problems locally.

Get usage instructions: .lint-whitespace.sh -?

Before this PR, the linenumber infomaition is output if trailing-space or tab code was found, but the output occurence is only per a file. This PR separates the output timing of file name and line number. As a result, users will find where they need to fix more easily.

@sipa

This enforces parts of the project header include guidelines (added by @sipa in #10575).

This partially reverts commit c36b720.

Zcash: Only the lint-* scripts we have backported.

Zcash: Only for lint-* scripts we have backported from upstream.

…using "export LC_ALL=C"

str4d · 2020-10-27T23:05:55Z

Rebased.

str4d · 2020-10-28T12:33:57Z

@zkbot try

zkbot · 2020-10-28T12:34:03Z

⌛ Trying commit 32fe883 with merge 43ac206...

daira

Flushing comments.

contrib/devtools/test-security-check.py

doc/developer-notes.md

test/lint/lint-locale-dependence.sh

doc/developer-notes.md

checksec.sh uses this format.

daira

Tested ACK with suggestions.

test/lint/lint-include-guards.sh

test/lint/lint-all.sh

test/lint/lint-includes.sh

test/lint/lint-python-utf8-encoding.sh

test/lint/lint-includes.sh

daira · 2020-10-28T13:45:16Z

test/lint/lint-locale-dependence.sh

+
+EXIT_CODE=0
+for LOCALE_DEPENDENT_FUNCTION in "${LOCALE_DEPENDENT_FUNCTIONS[@]}"; do
+    MATCHES=$(grep -E "[^a-zA-Z0-9_\`'\"<>]${LOCALE_DEPENDENT_FUNCTION}(_r|_s)?[^a-zA-Z0-9_\`'\"<>]" <<< "${GIT_GREP_OUTPUT}" | \


This doesn't check for a call, so it gives false positives when the name of a library function is shadowed by a local variable name. This only happens in one case: trim in src/crypto/equihash.h, so it might be just as easy to rename that variable.

test/lint/lint-shell.sh

test/lint/lint-whitespace.sh

test/lint/lint-shell.sh

test/lint/lint-shell-locale.sh

str4d · 2020-10-28T16:28:43Z

Not sure why Homu didn't report back, but the try passed.

Co-authored-by: Daira Hopwood <daira@jacaranda.org>

This instance is ok because it's guarded by a strict regex. Co-authored-by: Daira Hopwood <daira@jacaranda.org>

Either they don't violate the lints, or can be easily fixed. Co-authored-by: Daira Hopwood <daira@jacaranda.org>

str4d · 2020-11-09T20:07:39Z

@zkbot r+

zkbot · 2020-11-09T20:07:40Z

📌 Commit 1e6d183 has been approved by str4d

zkbot · 2020-11-09T21:21:00Z

⌛ Testing commit 1e6d183 with merge 84a5830...

zkbot · 2020-11-09T22:43:52Z

☀️ Test successful - pr-merge
Approved by: str4d
Pushing 84a5830 to master...

str4d added C-upstream-port Category: Changes that are ported from the Bitcoin Core codebase. A-CI Area: Continuous Integration labels Oct 27, 2020

str4d requested a review from daira October 27, 2020 12:37

Philip Kaufmann and others added 24 commits October 27, 2020 23:05

[Trivial] ensure minimal header conventions

70e1e11

- ensure header namespaces and end comments are correct - add missing header end comments - ensure minimal formatting (add newlines etc.)

Fix header guards using reserved identifiers

36463d4

Identifiers beginning with an underscore followed immediately by an uppercase letter are reserved.

Add tab char lint check and exclude imported dependencies

378d6d0

[scripts] lint-whitespace: use perl instead of grep -P

034fe80

MacOS does not support 'grep -P' out of the box. This change makes it easier for developers to check for whitespace problems locally.

[scripts] lint-whitespace: check last N commits or unstaged changes

c1683a1

Get usage instructions: .lint-whitespace.sh -?

devtools: Exclude patches from lint-whitespace

87f4e1c

Document include guard convention

653846b

Fix missing or inconsistent include guards

4ddfb21

Add lint-include-guards.sh which checks include guard consistency

87bc7fc

Add Travis check for duplicate includes

a3afa2a

This enforces parts of the project header include guidelines (added by @sipa in #10575).

Refine travis check for duplicate includes

5747638

This partially reverts commit c36b720.

Add shell script linting: Check for shellcheck warnings in shell scripts

eab2490

add lint tool to check python3 shebang

5d0affe

[contrib] convert test-security-check to python3

2788e33

test: Move linters to test/lint, add readme

c284807

Zcash: Only the lint-* scripts we have backported.

build: Guard against accidental introduction of new Boost dependencies

9174cca

build: Add linter for checking accidental locale dependence

40316e9

docs: Mention lint-locale-dependence.sh in developer-notes.md

aade01b

Add "export LC_ALL=C" to all shell scripts

745ac0a

Zcash: Only for lint-* scripts we have backported from upstream.

Add linter: Make sure all shell scripts opt out of locale dependence …

1b32acd

…using "export LC_ALL=C"

Explicitly specify encoding when opening text files in Python code

0eef155

lints: Use Zcash-specific include guards for new files

f318ab3

str4d force-pushed the backport-lints branch from 974a7eb to f318ab3 Compare October 27, 2020 23:05

lints: Update expected Boost imports

32fe883

daira reviewed Oct 28, 2020

View reviewed changes

daira mentioned this pull request Oct 28, 2020

Add to the list of locale-dependent functions that test/lint/lint-locale-dependence.sh checks for #4834

Open

practicalswift and others added 2 commits October 28, 2020 13:14

Follow-up to #13454: Fix broken build by exporting LC_ALL=C

69d3d38

lints: Match export LC_ALL="C" in lint-shell-locale

7c42009

checksec.sh uses this format.

daira approved these changes Oct 28, 2020

View reviewed changes

daira reviewed Oct 28, 2020

View reviewed changes

test/lint/lint-whitespace.sh Show resolved Hide resolved

daira reviewed Oct 28, 2020

View reviewed changes

test/lint/lint-shell.sh Show resolved Hide resolved

daira reviewed Oct 28, 2020

View reviewed changes

test/lint/lint-shell-locale.sh Outdated Show resolved Hide resolved

r3ld3v added this to the Core Sprint 2020-43 milestone Oct 28, 2020

r3ld3v modified the milestones: Core Sprint 2020-43, Core Sprint 2020-45 Nov 5, 2020

str4d and others added 4 commits November 9, 2020 17:15

Small documentation fixes

4b69b94

Co-authored-by: Daira Hopwood <daira@jacaranda.org>

lints: Add a missing copyright header

9dbf535

Co-authored-by: Daira Hopwood <daira@jacaranda.org>

lint: Allow stoi in src/rpc/blockchain.cpp

5b543b1

This instance is ok because it's guarded by a strict regex. Co-authored-by: Daira Hopwood <daira@jacaranda.org>

lint: Remove some subtrees from exclusion

1e6d183

Either they don't violate the lints, or can be easily fixed. Co-authored-by: Daira Hopwood <daira@jacaranda.org>

zkbot merged commit 84a5830 into zcash:master Nov 9, 2020

str4d deleted the backport-lints branch November 9, 2020 22:58

str4d added the S-committed Status: Planned work in a sprint label Nov 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Backport useful lints from upstream #4829

Backport useful lints from upstream #4829

str4d commented Oct 27, 2020 •

edited

Loading

str4d commented Oct 27, 2020 •

edited

Loading

str4d commented Oct 27, 2020

zkbot commented Oct 27, 2020

str4d commented Oct 27, 2020

str4d commented Oct 28, 2020

zkbot commented Oct 28, 2020

daira left a comment

daira left a comment

daira Oct 28, 2020

str4d commented Oct 28, 2020

str4d commented Nov 9, 2020

zkbot commented Nov 9, 2020

zkbot commented Nov 9, 2020

zkbot commented Nov 9, 2020

Backport useful lints from upstream #4829

Backport useful lints from upstream #4829

Conversation

str4d commented Oct 27, 2020 • edited Loading

str4d commented Oct 27, 2020 • edited Loading

str4d commented Oct 27, 2020

zkbot commented Oct 27, 2020

str4d commented Oct 27, 2020

str4d commented Oct 28, 2020

zkbot commented Oct 28, 2020

daira left a comment

Choose a reason for hiding this comment

daira left a comment

Choose a reason for hiding this comment

daira Oct 28, 2020

Choose a reason for hiding this comment

str4d commented Oct 28, 2020

str4d commented Nov 9, 2020

zkbot commented Nov 9, 2020

zkbot commented Nov 9, 2020

zkbot commented Nov 9, 2020

str4d commented Oct 27, 2020 •

edited

Loading

str4d commented Oct 27, 2020 •

edited

Loading