From 073969c044e902c039c812cf14da8b2cc55f860f Mon Sep 17 00:00:00 2001 From: Alex Apostolescu Date: Fri, 13 Dec 2024 16:39:37 +0200 Subject: [PATCH 1/5] infra/linters: Remove superlinter Superlinter is way to masive for our usecase and we should opt for smaller linters instead. Based on experience, we require: checkpatch, markdownlint, cpplint, black, and spellcheck. Signed-off-by: Alex Apostolescu --- .github/workflows/actions.yml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml index 9e25a1f3d7..d323de42bb 100644 --- a/.github/workflows/actions.yml +++ b/.github/workflows/actions.yml @@ -15,15 +15,6 @@ jobs: with: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - super-linter: - name: Super Linter - runs-on: ubuntu-latest - steps: - - name: Super Linter - uses: open-education-hub/actions/super-linter@main - with: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - spellcheck: name: Spellcheck runs-on: ubuntu-latest From 1961295fd7fab0f0963514f9f25fdffde68c1376 Mon Sep 17 00:00:00 2001 From: Alex Apostolescu Date: Fri, 13 Dec 2024 17:41:22 +0200 Subject: [PATCH 2/5] infra/linters: Add markdownlint Add markdownlint linter both as an action and as a Makefile rule. Signed-off-by: Alex Apostolescu --- .github/.markdownlint.yaml | 20 +++ .../common/inlineTokenChildren.js | 25 ++++ .../markdownlint-custom/common/wordPattern.js | 40 +++++ .github/markdownlint-custom/md101.js | 138 ++++++++++++++++++ .github/markdownlint-custom/md102.js | 59 ++++++++ .github/markdownlint-custom/md103.js | 21 +++ .github/markdownlint-custom/md104.js | 77 ++++++++++ .github/markdownlint-custom/rules.js | 9 ++ .github/workflows/actions.yml | 32 ++++ Makefile | 19 +++ 10 files changed, 440 insertions(+) create mode 100644 .github/.markdownlint.yaml create mode 100644 .github/markdownlint-custom/common/inlineTokenChildren.js create mode 100644 .github/markdownlint-custom/common/wordPattern.js create mode 100644 .github/markdownlint-custom/md101.js create mode 100644 .github/markdownlint-custom/md102.js create mode 100644 .github/markdownlint-custom/md103.js create mode 100644 .github/markdownlint-custom/md104.js create mode 100644 .github/markdownlint-custom/rules.js diff --git a/.github/.markdownlint.yaml b/.github/.markdownlint.yaml new file mode 100644 index 0000000000..2387b4322b --- /dev/null +++ b/.github/.markdownlint.yaml @@ -0,0 +1,20 @@ +# SPDX-License-Identifier: BSD-3-Clause +--- +default: true +MD004: false +MD010: false +MD013: false +MD024: false +MD029: + style: one +MD033: + allowed_elements: + - img + - br +MD034: false +MD035: false +MD041: false +MD046: + style: fenced +MD048: + style: backtick diff --git a/.github/markdownlint-custom/common/inlineTokenChildren.js b/.github/markdownlint-custom/common/inlineTokenChildren.js new file mode 100644 index 0000000000..bac5d106c8 --- /dev/null +++ b/.github/markdownlint-custom/common/inlineTokenChildren.js @@ -0,0 +1,25 @@ +class InlineTokenChildren { + constructor(token) { + if (token.type === "inline") { + this.root = token; + this.column = -1; + this.lineNumber = token.map[0]; + } else { + throw new TypeError("wrong argument token type"); + } + } + + *[Symbol.iterator]() { + for (let token of this.root.children) { + let { line, lineNumber } = token; + if (this.lineNumber !== lineNumber) { + this.column = -1; + this.lineNumber = lineNumber; + } + this.column = line.indexOf(token.content, this.column + 1); + yield { token, column: this.column + 1, lineNumber }; + } + } +} + +module.exports = { InlineTokenChildren }; diff --git a/.github/markdownlint-custom/common/wordPattern.js b/.github/markdownlint-custom/common/wordPattern.js new file mode 100644 index 0000000000..58bf1e8e9e --- /dev/null +++ b/.github/markdownlint-custom/common/wordPattern.js @@ -0,0 +1,40 @@ +class WordPattern { + constructor(pattern, parameters) { + const escapedDots = pattern.replace(/\\?\./g, "\\."); + this.pattern = parameters && parameters.hasOwnProperty('noWordBoundary') ? escapedDots : "\\b" + escapedDots + "\\b"; + const modifiers = parameters && parameters.hasOwnProperty('caseSensitive') && parameters.caseSensitive ? "" : "i"; + this.regex = new RegExp(this.pattern, modifiers); + this.suggestion = parameters && parameters.hasOwnProperty('suggestion') ? parameters.suggestion : pattern; + this.stringRegex = new RegExp("^" + escapedDots + "$", modifiers); // To match "Category" column words in changelogs, see case-sensitive.js + this.skipForUseCases = !!(parameters && parameters.hasOwnProperty('skipForUseCases')); + } + + test(line) { + return new Match(line.match(this.regex)); + } +} + +class Match { + constructor(match) { + this.match = match; + } + + range() { + if (this.match) { + let column = this.match.index + 1; + let length = this.match[0].length; + if (this.match[2]) { + column += this.match[1].length; + length -= this.match[1].length; + } + return [column, length]; + } + return null; + } + + toString() { + return this.match ? this.match.toString() : "null"; + } +} + +module.exports = { WordPattern }; diff --git a/.github/markdownlint-custom/md101.js b/.github/markdownlint-custom/md101.js new file mode 100644 index 0000000000..f82fe16830 --- /dev/null +++ b/.github/markdownlint-custom/md101.js @@ -0,0 +1,138 @@ +const { InlineTokenChildren } = require("./common/inlineTokenChildren"); +const { WordPattern } = require("./common/wordPattern"); + +const keywords = [ + new WordPattern("curl"), + new WordPattern("wget"), + new WordPattern("crontab"), + new WordPattern("cron"), + new WordPattern("netcat"), + new WordPattern("ping"), + new WordPattern("traceroute"), + new WordPattern("sudo"), + new WordPattern("jps"), + new WordPattern("name=value"), + new WordPattern("key=value"), + new WordPattern("time:value"), + new WordPattern("atsd.log"), + new WordPattern("start.log"), + new WordPattern("logback.xml"), + new WordPattern("graphite.conf"), + new WordPattern("command_malformed.log"), + new WordPattern("stdout"), + new WordPattern("stdin"), + new WordPattern("stderr"), + new WordPattern("SIGHUP"), + new WordPattern("SIGINT"), + new WordPattern("SIGQUIT"), + new WordPattern("SIGILL"), + new WordPattern("SIGTRAP"), + new WordPattern("SIGABRT"), + new WordPattern("SIGBUS"), + new WordPattern("SIGFPE"), + new WordPattern("SIGKILL"), + new WordPattern("SIGUSR1"), + new WordPattern("SIGSEGV"), + new WordPattern("SIGUSR2"), + new WordPattern("SIGPIPE"), + new WordPattern("SIGALRM"), + new WordPattern("SIGTERM"), + new WordPattern("SIGSTKFLT"), + new WordPattern("SIGCHLD"), + new WordPattern("SIGCONT"), + new WordPattern("SIGSTOP"), + new WordPattern("SIGTSTP"), + new WordPattern("SIGTTIN"), + new WordPattern("SIGTTOU"), + new WordPattern("SIGURG"), + new WordPattern("SIGXCPU"), + new WordPattern("SIGXFSZ"), + new WordPattern("SIGVTALRM"), + new WordPattern("SIGPROF"), + new WordPattern("SIGWINCH"), + new WordPattern("SIGIO"), + new WordPattern("SIGPWR"), + new WordPattern("SIGSYS"), + new WordPattern("NaN"), + new WordPattern(".png", { noWordBoundary: true }), + new WordPattern(".xml", { noWordBoundary: true }), + new WordPattern(".svg", { noWordBoundary: true }), + new WordPattern(".jar", { noWordBoundary: true }), + new WordPattern(".gz", { noWordBoundary: true }), + new WordPattern(".tar.gz", { noWordBoundary: true }), + new WordPattern(".bz2", { noWordBoundary: true }), + new WordPattern(".tar.bz2", { noWordBoundary: true }), + new WordPattern(".zip", { noWordBoundary: true }), + new WordPattern(".txt", { noWordBoundary: true }), + new WordPattern(".tex", { noWordBoundary: true }), + new WordPattern(".yaml", { noWordBoundary: true }), + new WordPattern(".csv", { noWordBoundary: true }), + new WordPattern(".json", { noWordBoundary: true }), + new WordPattern(".pdf", { noWordBoundary: true }), + new WordPattern(".html", { noWordBoundary: true }), + new WordPattern(".c", { noWordBoundary: true }), + new WordPattern(".cpp", { noWordBoundary: true }), + new WordPattern(".h", { noWordBoundary: true }), + new WordPattern(".hpp", { noWordBoundary: true }), + new WordPattern(".py", { noWordBoundary: true }), + new WordPattern(".rs", { noWordBoundary: true }), + new WordPattern(".d", { noWordBoundary: true }), + new WordPattern(".go", { noWordBoundary: true }), + new WordPattern(".js", { noWordBoundary: true }), + new WordPattern(".java", { noWordBoundary: true }), + new WordPattern(".go", { noWordBoundary: true }), + new WordPattern(".php", { noWordBoundary: true }), + new WordPattern(".pl", { noWordBoundary: true }), + new WordPattern(".rb", { noWordBoundary: true }), + new WordPattern(".o", { noWordBoundary: true }), + new WordPattern(".a", { noWordBoundary: true }), + new WordPattern(".so", { noWordBoundary: true }), + new WordPattern(".obj", { noWordBoundary: true }), + new WordPattern(".lib", { noWordBoundary: true }), + new WordPattern(".dll", { noWordBoundary: true }), + new WordPattern(".dylib", { noWordBoundary: true }), +]; + +module.exports = { + names: ["MD101", "backtick-keywords"], + description: "Keywords must be fenced and must be in appropriate case.", + tags: ["backtick", "code", "bash"], + "function": (params, onError) => { + var inHeading = false; + var inLink = false; + for (let token of params.tokens) { + switch (token.type) { + case "heading_open": + inHeading = true; break; + case "heading_close": + inHeading = false; break; + case "inline": + let children = new InlineTokenChildren(token); + for (let { token: child, column, lineNumber } of children) { + let isText = child.type === "text"; + switch (child.type) { + case "link_open": + inLink = true; break; + case "link_close": + inLink = false; break; + } + for (let k of keywords) { + let anyCaseMatch = child.content.match(k.regex); + if (anyCaseMatch != null) { + let match = anyCaseMatch[0]; + let correct = k.suggestion; + if ((!inHeading && !inLink && isText) || // Bad not fenced + (match !== correct)) { // Right fencing, wrong case + onError({ + lineNumber, + detail: `Expected \`${correct}\`. Actual ${match}.`, + range: [column + anyCaseMatch.index, match.length] + }) + } + } + } + } + } + } + } +}; diff --git a/.github/markdownlint-custom/md102.js b/.github/markdownlint-custom/md102.js new file mode 100644 index 0000000000..5a2686855a --- /dev/null +++ b/.github/markdownlint-custom/md102.js @@ -0,0 +1,59 @@ +const http_keywords = [ + "GET", + "POST", + "PUT", + "PATCH", + "DELETE", + "Content-Type", + "Content-Encoding", + "User-Agent", + "200 OK", + "401 Unauthorized", + "403 Forbidden", + "404 Not Found", + "API_DATA_READ", + "API_DATA_WRITE", + "API_META_READ", + "API_META_WRITE", + "USER", + "EDITOR", + "ENTITY_GROUP_ADMIN", + "ADMIN" +]; +const keywordsRegex = new RegExp(http_keywords.map(word => "\\b" + word + "\\b").join("|")); + +const { InlineTokenChildren } = require("./common/inlineTokenChildren"); + +module.exports = { + names: ["MD102", "backtick-http"], + description: "HTTP keywords must be fenced.", + tags: ["backtick", "HTTP", "HTTPS"], + "function": (params, onError) => { + var inHeading = false; + for (let token of params.tokens) { + switch (token.type) { + case "heading_open": + inHeading = true; break; + case "heading_close": + inHeading = false; break; + case "inline": + if (!inHeading) { + let children = new InlineTokenChildren(token); + for (let { token: child, column, lineNumber } of children) { + if (child.type === "text") { + let exactCaseMatch = child.content.match(keywordsRegex); + if (exactCaseMatch != null) { + let match = exactCaseMatch[0]; + onError({ + lineNumber, + detail: `Expected \`${match}\`. Actual ${match}.`, + range: [column + exactCaseMatch.index, match.length] + }) + } + } + } + } + } + } + } +}; diff --git a/.github/markdownlint-custom/md103.js b/.github/markdownlint-custom/md103.js new file mode 100644 index 0000000000..aed2a30429 --- /dev/null +++ b/.github/markdownlint-custom/md103.js @@ -0,0 +1,21 @@ +"use strict"; + +module.exports = { + "names": [ "MD103", "inline triple backticks" ], + "description": "inline triple backticks", + "tags": [ "backticks" ], + "function": function rule(params, onError) { + for (const inline of params.tokens.filter(function filterToken(token) { + return token.type === "inline"; + })) { + const index = inline.content.toLowerCase().indexOf("```"); + if (index !== -1) { + onError({ + "lineNumber": inline.lineNumber, + "context": inline.content.substr(index - 1, 4), + "detail": "Expected `. Actual ```" + }); + } + } + } +}; diff --git a/.github/markdownlint-custom/md104.js b/.github/markdownlint-custom/md104.js new file mode 100644 index 0000000000..044610f5db --- /dev/null +++ b/.github/markdownlint-custom/md104.js @@ -0,0 +1,77 @@ +"use strict"; + +function strip_words(line) { + const URL_REGEX = new RegExp(/https?:\/\/(www\.)?[-a-zA-Z0-9@:%._+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b([-a-zA-Z0-9()@:%_+.~#?&//=]*)?/gi); + const QUOTED_WORD_REGEX = new RegExp(/\`(.*?)\`|\"(.*?)\"/g) + const FLOAT_REGEX = new RegExp(/[-+]?\b\d+\.\d+\b/g) + const DATE_REGEX = new RegExp(/\b\d{2}\.\d{2}\.\d{4}\b/g) + const IGNORED_REGEXES = [ + URL_REGEX, + QUOTED_WORD_REGEX, + FLOAT_REGEX, + DATE_REGEX, + 'i.e.', + 'e.g.', + 'etc.', + 'vs.', + '...' + ] + + IGNORED_REGEXES.forEach((i_word) => line = line.replace(i_word, '')) + + return line +} + +module.exports = { + names: ["MD104", "one line per sentence"], + description: "one line (and only one line) per sentence", + tags: ["sentences"], + function: function rule(params, onError) { + for (const inline of params.tokens.filter(function filterToken(token) { + return token.type === "inline"; + })) { + var actual_lines = inline.content.split("\n"); + actual_lines.forEach((line, index, arr) => { + line = strip_words(line) + let outside = true; + let count = 0; + let escaped = false; + let prev_dollar = false + let latex_inline = false; + let latex_block = false; + Array.from(line).forEach((char) => { + if (char == "\\") + escaped = true; + else + escaped = false; + + if ((char == "$") && !escaped && outside) { + if (prev_dollar) { + latex_block = !latex_block; + prev_dollar = false + } else { + latex_inline = !latex_inline; + prev_dollar = true; + } + } + + if ((char == "." || char == "?" || char == "!" || char == ";") && outside && !latex_block && !latex_inline) { + count++; + } + if (char == "`") outside = !outside; + if (char == "[") outside = false; + if (char == "(") outside = false; + if (char == "]") outside = true; + if (char == ")") outside = true; + }); + if (count > 1) { + onError({ + lineNumber: inline.lineNumber + index, + detail: + "Expected one sentence per line. Multiple end of sentence punctuation signs found on one line!", + }); + } + }); + } + }, +}; diff --git a/.github/markdownlint-custom/rules.js b/.github/markdownlint-custom/rules.js new file mode 100644 index 0000000000..ced23cf7db --- /dev/null +++ b/.github/markdownlint-custom/rules.js @@ -0,0 +1,9 @@ +"use strict"; + +const rules = [ + require("./md101.js"), + require("./md102.js"), + require("./md103.js"), + require("./md104.js"), +]; +module.exports = rules; diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml index d323de42bb..188cf81cdc 100644 --- a/.github/workflows/actions.yml +++ b/.github/workflows/actions.yml @@ -23,3 +23,35 @@ jobs: uses: open-education-hub/actions/spellcheck@main with: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + markdownlint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Get changed markdown files + uses: tj-actions/changed-files@v45 + id: changed-files + with: + files: "**/*.md" + separator: " " + + - name: Setup Node + uses: actions/setup-node@v4 + with: + node-version: 20 + + - run: npm install -g markdownlint-cli@0.43.0 + - env: + MARKDOWNLINT_CONFIG: ".github/.markdownlint.yaml" + MARKDOWNLINT_RULES: ".github/markdownlint-custom/rules.js" + # Markdownlint will fail if no arguments are provided. + # In that case check README.md. + FILES: ${{ steps.changed-files.outputs.all_changed_files || 'README.md' }} + run: + markdownlint + --config ${{ env.MARKDOWNLINT_CONFIG }} + --rules ${{ env.MARKDOWNLINT_RULES }} + ${{ env.FILES }} diff --git a/Makefile b/Makefile index f9ae686d4c..2cabe0248b 100644 --- a/Makefile +++ b/Makefile @@ -39,3 +39,22 @@ clean: stop_bash cleanall: clean -docker inspect --type=image $(IMAGE_NAME) > /dev/null 2>&1 && docker image rm $(IMAGE_NAME) + +.PHONY: lint markdownlint +lint: markdownlint + +CHANGED_MD_FILES = $(call get_changed_files,\.md$$) +markdownlint: + @echo "Checking markdown files ..." + @if [ "$(CHANGED_MD_FILES)" ]; then \ + docker run --rm -v $(PWD):/md peterdavehello/markdownlint markdownlint \ + --config .github/.markdownlint.yaml \ + --rules .github/markdownlint-custom/rules.js \ + $(CHANGED_MD_FILES); \ + fi + + +# Get a list of files that changed in the current branch and match the given pattern +define get_changed_files # $(1): pattern +$(shell git diff --name-only HEAD $(shell git merge-base HEAD main) | grep "$(1)" | tr '\n' ' ') +endef From 833300a843f79ee7f2823f1eb8835e736d9f9d2a Mon Sep 17 00:00:00 2001 From: Alex Apostolescu Date: Fri, 13 Dec 2024 19:41:56 +0200 Subject: [PATCH 3/5] infra/linters: Add cpplint Add cpplint linter both as an action and as a Makefile rule. I was unable to find a docker image to run cpplint so for the moment it runs locally. Signed-off-by: Alex Apostolescu --- .github/workflows/actions.yml | 26 +++++++++++++++++++ CPPLINT.cfg | 1 + Makefile | 10 +++++-- .../support/CPPLINT.cfg | 4 +-- .../common-functions/solution/src/CPPLINT.cfg | 3 ++- 5 files changed, 39 insertions(+), 5 deletions(-) diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml index 188cf81cdc..d8c643c781 100644 --- a/.github/workflows/actions.yml +++ b/.github/workflows/actions.yml @@ -55,3 +55,29 @@ jobs: --config ${{ env.MARKDOWNLINT_CONFIG }} --rules ${{ env.MARKDOWNLINT_RULES }} ${{ env.FILES }} + + cpplint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Get changed C and C++ files + uses: tj-actions/changed-files@v45 + id: changed-files + with: + files: "**/*.{c,cc,cpp,h,hpp}" + separator: " " + + - uses: actions/setup-python@v5 + with: + python-version: 3.12 + + - run: pip install cpplint + - env: + FILES: ${{ steps.changed-files.outputs.all_changed_files }} + run: | + if [[ -n "${{ env.FILES }}" ]]; then + cpplint --recursive --quiet ${{ env.FILES }} + fi diff --git a/CPPLINT.cfg b/CPPLINT.cfg index 3241f6cd1e..acd72655eb 100644 --- a/CPPLINT.cfg +++ b/CPPLINT.cfg @@ -5,6 +5,7 @@ filter=-whitespace/semicolon filter=-legal/copyright filter=-whitespace/comments filter=-build/header_guard +filter=-build/include_what_you_use filter=-readability/multiline_comment filter=-readability/casting filter=-runtime/int diff --git a/Makefile b/Makefile index 2cabe0248b..b9f2c9599a 100644 --- a/Makefile +++ b/Makefile @@ -40,8 +40,8 @@ clean: stop_bash cleanall: clean -docker inspect --type=image $(IMAGE_NAME) > /dev/null 2>&1 && docker image rm $(IMAGE_NAME) -.PHONY: lint markdownlint -lint: markdownlint +.PHONY: lint markdownlint cpplint +lint: markdownlint cpplint CHANGED_MD_FILES = $(call get_changed_files,\.md$$) markdownlint: @@ -53,6 +53,12 @@ markdownlint: $(CHANGED_MD_FILES); \ fi +CHANGED_CPP_FILES = $(call get_changed_files,\.\(c\|cc\|cpp\|h\|hpp\)$$) +cpplint: # Runs locally, requires cpplint installed + @echo "Checking C/C++ files ..." + @if [ "$(CHANGED_CPP_FILES)" ]; then \ + cpplint --recursive --quiet $(CHANGED_CPP_FILES); \ + fi # Get a list of files that changed in the current branch and match the given pattern define get_changed_files # $(1): pattern diff --git a/chapters/compute/synchronization/drills/tasks/threadsafe-data-struct/support/CPPLINT.cfg b/chapters/compute/synchronization/drills/tasks/threadsafe-data-struct/support/CPPLINT.cfg index c7316966d9..ae04809338 100644 --- a/chapters/compute/synchronization/drills/tasks/threadsafe-data-struct/support/CPPLINT.cfg +++ b/chapters/compute/synchronization/drills/tasks/threadsafe-data-struct/support/CPPLINT.cfg @@ -1,2 +1,2 @@ -exclude_files=clist\.(c|h) -exclude_files=test\.c +# Exclude 3rd party code +exclude_files=.*\.(c|h)$ diff --git a/chapters/software-stack/libc/drills/tasks/common-functions/solution/src/CPPLINT.cfg b/chapters/software-stack/libc/drills/tasks/common-functions/solution/src/CPPLINT.cfg index a1ba239424..ae04809338 100644 --- a/chapters/software-stack/libc/drills/tasks/common-functions/solution/src/CPPLINT.cfg +++ b/chapters/software-stack/libc/drills/tasks/common-functions/solution/src/CPPLINT.cfg @@ -1 +1,2 @@ -exclude_files=printf\.(c|h) +# Exclude 3rd party code +exclude_files=.*\.(c|h)$ From 339cb010e1bdf8c502bf5964cc841596227600b6 Mon Sep 17 00:00:00 2001 From: Alex Apostolescu Date: Fri, 13 Dec 2024 23:51:00 +0200 Subject: [PATCH 4/5] infra/linters: Add black Add black python linter both as an action and as a Makefile rule. Signed-off-by: Alex Apostolescu --- .github/workflows/actions.yml | 27 +++++++++++++++++++++++++++ Makefile | 12 ++++++++++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml index d8c643c781..4d4bab69bb 100644 --- a/.github/workflows/actions.yml +++ b/.github/workflows/actions.yml @@ -81,3 +81,30 @@ jobs: if [[ -n "${{ env.FILES }}" ]]; then cpplint --recursive --quiet ${{ env.FILES }} fi + + black: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Get changed Python files + uses: tj-actions/changed-files@v45 + id: changed-files + with: + files: "**/*.py" + separator: " " + + - uses: actions/setup-python@v5 + with: + python-version: 3.12 + + - uses: psf/black@stable + env: + # Black will fail if no arguments are provided. + # In that case check a directory without any python files. + FILES: ${{ steps.changed-files.outputs.all_changed_files || '.github' }} + with: + options: "--check --diff" + src: ${{ env.FILES }} diff --git a/Makefile b/Makefile index b9f2c9599a..55240f987e 100644 --- a/Makefile +++ b/Makefile @@ -40,8 +40,8 @@ clean: stop_bash cleanall: clean -docker inspect --type=image $(IMAGE_NAME) > /dev/null 2>&1 && docker image rm $(IMAGE_NAME) -.PHONY: lint markdownlint cpplint -lint: markdownlint cpplint +.PHONY: lint markdownlint cpplint black +lint: markdownlint cpplint black CHANGED_MD_FILES = $(call get_changed_files,\.md$$) markdownlint: @@ -60,6 +60,14 @@ cpplint: # Runs locally, requires cpplint installed cpplint --recursive --quiet $(CHANGED_CPP_FILES); \ fi +CHANGED_PY_FILES = $(call get_changed_files,\.py$$) +black: + @echo "Checking Python files ..." + @if [ "$(CHANGED_PY_FILES)" ]; then \ + docker run --rm -v $(PWD):/data --workdir=/data pyfound/black:latest \ + black --check --diff $(CHANGED_PY_FILES); \ + fi + # Get a list of files that changed in the current branch and match the given pattern define get_changed_files # $(1): pattern $(shell git diff --name-only HEAD $(shell git merge-base HEAD main) | grep "$(1)" | tr '\n' ' ') From 4f4111f15dd1a21d91abdef7abf32c5a861e6dd9 Mon Sep 17 00:00:00 2001 From: Alex Apostolescu Date: Sat, 14 Dec 2024 01:30:07 +0200 Subject: [PATCH 5/5] infra/linters: Add shellcheck Add shellcheck linter both as an action and as a Makefile rule. Fail on severity greater than 'warning' to avoid refactoring current scripts. Signed-off-by: Alex Apostolescu --- .github/workflows/actions.yml | 13 +++++++++++++ Makefile | 12 ++++++++++-- .../lambda-function-loader/tests/checker.sh | 5 ++++- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml index 4d4bab69bb..afd3ab4999 100644 --- a/.github/workflows/actions.yml +++ b/.github/workflows/actions.yml @@ -108,3 +108,16 @@ jobs: with: options: "--check --diff" src: ${{ env.FILES }} + + shellcheck: # Runs on all shell scripts in the repository. + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: shellcheck + uses: ludeeus/action-shellcheck@2.0.0 + with: + version: v0.10.0 + severity: "warning" diff --git a/Makefile b/Makefile index 55240f987e..3ee6abb2ad 100644 --- a/Makefile +++ b/Makefile @@ -40,8 +40,8 @@ clean: stop_bash cleanall: clean -docker inspect --type=image $(IMAGE_NAME) > /dev/null 2>&1 && docker image rm $(IMAGE_NAME) -.PHONY: lint markdownlint cpplint black -lint: markdownlint cpplint black +.PHONY: lint markdownlint cpplint black shellcheck +lint: markdownlint cpplint black shellcheck CHANGED_MD_FILES = $(call get_changed_files,\.md$$) markdownlint: @@ -68,6 +68,14 @@ black: black --check --diff $(CHANGED_PY_FILES); \ fi +CHANGED_SHELL_FILES = $(call get_changed_files,\.\(sh\|bash\)$$) +shellcheck: + @echo "Checking shell scripts ..." + @if [ "$(CHANGED_SHELL_FILES)" ]; then \ + docker run --rm -v $(PWD):/data --workdir=/data koalaman/shellcheck-alpine:v0.10.0 \ + shellcheck --severity=warning $(CHANGED_SHELL_FILES); \ + fi + # Get a list of files that changed in the current branch and match the given pattern define get_changed_files # $(1): pattern $(shell git diff --name-only HEAD $(shell git merge-base HEAD main) | grep "$(1)" | tr '\n' ' ') diff --git a/content/assignments/lambda-function-loader/tests/checker.sh b/content/assignments/lambda-function-loader/tests/checker.sh index d1af4b57ba..b1adbca54b 100755 --- a/content/assignments/lambda-function-loader/tests/checker.sh +++ b/content/assignments/lambda-function-loader/tests/checker.sh @@ -501,7 +501,10 @@ main() { run_tests "$@" if [[ $# -eq 0 ]]; then - PADDING=$(printf '.%.0s' $(eval echo "{1..$((PADDING_LEN+6))}")) + PADDING="" + for ((i=0; i<((PADDING_LEN+6)); i++)); do + PADDING+="." + done printf "Total Score%s[%+3s/%-3s]\n" "$PADDING" $TOTAL_SCORE $MAX_TOTAL_SCORE fi }