feat(descriptors, lua): add `selene` support #3978

AlejandroSuero · 2024-09-09T10:37:37Z

Added selene lua linter support to lua descriptors.
#3971

Fixes #

Proposed Changes

Added selene to descriptors

Readiness Checklist

Author/Contributor

Add entry to the CHANGELOG listing the change and linking to the corresponding issue (if appropriate)
If documentation is needed for this change, has that been included in this pull request

Reviewing Maintainer

Label as breaking if this is a large fundamental change
Label as either automation, bug, documentation, enhancement, infrastructure, or performance

echoix · 2024-09-09T11:30:44Z

@nvuillam and what is needed as test file for a new linter?

echoix · 2024-09-09T11:30:51Z

/build

Command run output
Build command workflow started.
Installing dependencies
Running script ./build.sh

github-actions · 2024-09-09T11:34:13Z

Build command workflow failed.

echoix · 2024-09-09T11:59:25Z

Since updating the repo from the new linter modifies some workflow files, I need a human to run the build command.

If you are on Linux, it is ./build.sh, otherwise,

pip install pipx
pipx install hatch
hatch shell

hatch run build:build

AlejandroSuero · 2024-09-09T13:02:40Z

@echoix I ran it from a MacOS and ./build.sh error because import git. After hatch run build:build it said import jsonschema when running ./build.sh.

After hatch run build:build:

Do I commit all these files, right?

echoix · 2024-09-09T13:06:04Z

Yep, the same files that you can see in the logs of the workflow I tried to do automatically, but couldn't. (We don't add new linters often nowadays, so it wasn't expected to work everywhere)

nvuillam · 2024-09-09T17:30:52Z

@nvuillam and what is needed as test file for a new linter?

About tests, let's see if the ones existing for luacheck do the work :)

https://github.com/oxsecurity/megalinter/tree/main/.automation/test/lua

nvuillam · 2024-09-09T17:34:47Z

@AlejandroSuero , I think you can factorize the installation of lua at descriptor level (install property also defined at top level )

And keep at linter level only what is purely related to the linter :)

Example with powershell: https://github.com/oxsecurity/megalinter/blob/main/megalinter/descriptors/powershell.megalinter-descriptor.yml

AlejandroSuero · 2024-09-09T17:41:37Z

@nvuillam here are the results for selene in existing tests:

nvuillam · 2024-09-09T17:43:48Z

@AlejandroSuero can you update the test cases so they work with both luacheck and selene ?

Otherwise, it's possible to create a test cases folder just for lua_selene and set property test_folder in selene descriptor, but if we can avoid that I prefer :)

AlejandroSuero · 2024-09-09T17:47:26Z

@AlejandroSuero , I think you can factorize the installation of lua at descriptor level (install property also defined at top level )

And keep at linter level only what is purely related to the linter :)

Example with powershell: https://github.com/oxsecurity/megalinter/blob/main/megalinter/descriptors/powershell.megalinter-descriptor.yml

On the powershell one the top-level install it installs powershell itself, but then the installs on the linters are separated from that.

Do I move the luacheck install and selene install into the same one in the top-level?

AlejandroSuero · 2024-09-09T17:54:25Z

With the commit 442fbf7 the test results are:

Both luacheck and selene fail in lua_bad_1.lua and pass in lua_good_1.lua as expected.

echoix · 2024-09-09T17:58:16Z

@AlejandroSuero , I think you can factorize the installation of lua at descriptor level (install property also defined at top level )

And keep at linter level only what is purely related to the linter :)

Example with powershell: https://github.com/oxsecurity/megalinter/blob/main/megalinter/descriptors/powershell.megalinter-descriptor.yml

The next linter (StyLua) is a Docker copy-only linter though, so in the individual linter image, the language-level install will be suboptimal.

megalinter/descriptors/lua.megalinter-descriptor.yml

echoix · 2024-09-09T20:09:34Z

megalinter/descriptors/lua.megalinter-descriptor.yml

+        - g++
+      dockerfile:
+        - |
+          RUN cargo install --branch main --git https://github.com/Kampfkarren/selene selene


We try to use official releases, so maybe try the following:

Suggested change

RUN cargo install --branch main --git https://github.com/Kampfkarren/selene selene

RUN cargo install selene@0.27.1

Im still trying to find the correct syntax to keep the version number updated

For what I saw with cargo in the rust descriptor, it's just being omitted (just using the crate name):

megalinter/megalinter/descriptors/rust.megalinter-descriptor.yml

Lines 23 to 25 in 8ec5e3e

install:

cargo:

- clippy

Same with npm packages:

megalinter/megalinter/descriptors/javascript.megalinter-descriptor.yml

Lines 58 to 72 in 8ec5e3e

install:

npm:

- eslint

- eslint-config-airbnb

- eslint-config-prettier

- eslint-config-standard

- eslint-plugin-import

- eslint-plugin-jest

- eslint-plugin-node

- eslint-plugin-prettier

- eslint-plugin-promise

- eslint-plugin-vue

- "@babel/core"

- "@babel/eslint-parser"

- "@microsoft/eslint-formatter-sarif"

But different in here, using ARG:

megalinter/megalinter/descriptors/editorconfig.megalinter-descriptor.yml

Lines 26 to 32 in 8ec5e3e

install:

dockerfile:

- |-

# renovate: datasource=docker depName=mstruebing/editorconfig-checker

ARG EDITORCONFIG_EDITORCONFIG_CHECKER_VERSION=v3.0.3

- FROM mstruebing/editorconfig-checker:${EDITORCONFIG_EDITORCONFIG_CHECKER_VERSION} AS editorconfig-checker

- COPY --link --from=editorconfig-checker /usr/bin/ec /usr/bin/editorconfig-checker

We weren't pinning packages up until a couple weeks ago, from my push to make keeping them updatable manageable. So you'll find lots of places where they aren't implemented yet

Do I leave it with [email protected] or just selene for the time being.

We can try with this: #3978 (comment)

@echoix LGTM 👍

echoix · 2024-09-09T20:16:05Z

megalinter/descriptors/lua.megalinter-descriptor.yml

+        - g++
+      dockerfile:
+        - |
+          RUN cargo install --branch main --git https://github.com/Kampfkarren/selene selene


@nvuillam where does cargo get set up in individual linters? As it isn't being built in the PR, we don't see it fail yet

echoix · 2024-09-09T20:18:17Z

megalinter/descriptors/lua.megalinter-descriptor.yml

+        - g++
+      dockerfile:
+        - |
+          RUN cargo install --branch main --git https://github.com/Kampfkarren/selene selene


Im still trying to find the correct syntax to keep the version number updated

megalinter/descriptors/lua.megalinter-descriptor.yml

Co-authored-by: Edouard Choinière <[email protected]>

echoix · 2024-09-09T21:31:25Z

/build

Command run output
Build command workflow started.
Installing dependencies
Running script ./build.sh
Build command workflow completed updating files.

echoix · 2024-09-09T22:08:11Z

I'm running tests of individual linters in echoix#120

megalinter/descriptors/lua.megalinter-descriptor.yml

echoix · 2024-09-10T10:51:52Z

Thanks again! I assure you, the StyLua is way easier to integrate, it's only a copy operation

nvuillam · 2024-09-10T10:55:12Z

Great job @AlejandroSuero , i'll mention you in the LinkedIn post at the next MegaLinter release 😊

AlejandroSuero · 2024-09-10T11:02:02Z

@nvuillam @echoix thanks for the feedback and the help.

Should this be the way to copy it?

https://github.com/JohnnyMorganz/StyLua?tab=readme-ov-file#docker

https://hub.docker.com/r/johnnymorganz/stylua/tags

echoix · 2024-09-10T12:05:03Z

Yes, but I think there's missing the $ for using the var

feat(descriptors, lua): add selene support

c139477

AlejandroSuero requested review from nvuillam, echoix, bdovaz and Kurt-von-Laven as code owners September 9, 2024 10:37

AlejandroSuero temporarily deployed to dev September 9, 2024 11:28 — with GitHub Actions Inactive

AlejandroSuero had a problem deploying to dev September 9, 2024 11:28 — with GitHub Actions Failure

feat: build process

57ccde0

AlejandroSuero temporarily deployed to dev September 9, 2024 13:11 — with GitHub Actions Inactive

AlejandroSuero had a problem deploying to dev September 9, 2024 13:11 — with GitHub Actions Failure

AlejandroSuero added 2 commits September 9, 2024 15:25

fix: remove non-existent idea plugin

a38c9f0

fix: cspell fixes

f166d08

AlejandroSuero temporarily deployed to dev September 9, 2024 17:29 — with GitHub Actions Inactive

fix(test): working test for selene and luacheck

442fbf7

AlejandroSuero had a problem deploying to dev September 9, 2024 17:57 — with GitHub Actions Error

AlejandroSuero temporarily deployed to dev September 9, 2024 17:57 — with GitHub Actions Inactive

echoix requested changes Sep 9, 2024

View reviewed changes

megalinter/descriptors/lua.megalinter-descriptor.yml Outdated Show resolved Hide resolved

echoix requested changes Sep 9, 2024

View reviewed changes

echoix reviewed Sep 9, 2024

View reviewed changes

echoix requested changes Sep 9, 2024

View reviewed changes

megalinter/descriptors/lua.megalinter-descriptor.yml Outdated Show resolved Hide resolved

AlejandroSuero and others added 2 commits September 9, 2024 22:58

fix: remove next command \ jump

0795361

Co-authored-by: Edouard Choinière <[email protected]>

feat: use cargo crates

a58c0cc

Co-authored-by: Edouard Choinière <[email protected]>

AlejandroSuero temporarily deployed to dev September 9, 2024 21:08 — with GitHub Actions Inactive

[build-command] Update generated files

498b233

echoix closed this Sep 9, 2024

echoix reopened this Sep 9, 2024

echoix temporarily deployed to dev September 9, 2024 21:36 — with GitHub Actions Inactive

feat: use dockerfile with LUA_SELENE_VERSION

7c6def6

AlejandroSuero temporarily deployed to dev September 9, 2024 21:55 — with GitHub Actions Inactive

echoix requested changes Sep 10, 2024

View reviewed changes

megalinter/descriptors/lua.megalinter-descriptor.yml Outdated Show resolved Hide resolved

fix: include readline-dev in language-level

ecf8a2c

AlejandroSuero temporarily deployed to dev September 10, 2024 09:50 — with GitHub Actions Inactive

echoix approved these changes Sep 10, 2024

View reviewed changes

echoix enabled auto-merge (squash) September 10, 2024 10:08

echoix merged commit 64261ae into oxsecurity:main Sep 10, 2024
6 checks passed

AlejandroSuero deleted the add-lua-linter-selene branch September 10, 2024 10:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(descriptors, lua): add `selene` support #3978

feat(descriptors, lua): add `selene` support #3978

AlejandroSuero commented Sep 9, 2024

echoix commented Sep 9, 2024

echoix commented Sep 9, 2024 •

edited by github-actions bot

Loading

github-actions bot commented Sep 9, 2024

echoix commented Sep 9, 2024

AlejandroSuero commented Sep 9, 2024

echoix commented Sep 9, 2024

nvuillam commented Sep 9, 2024

nvuillam commented Sep 9, 2024

AlejandroSuero commented Sep 9, 2024

nvuillam commented Sep 9, 2024

AlejandroSuero commented Sep 9, 2024

AlejandroSuero commented Sep 9, 2024

echoix commented Sep 9, 2024 •

edited

Loading

echoix Sep 9, 2024

echoix Sep 9, 2024

AlejandroSuero Sep 9, 2024 •

edited

Loading

echoix Sep 9, 2024

AlejandroSuero Sep 9, 2024

echoix Sep 9, 2024

AlejandroSuero Sep 9, 2024

echoix Sep 9, 2024

echoix Sep 9, 2024

echoix commented Sep 9, 2024 •

edited by github-actions bot

Loading

echoix commented Sep 9, 2024

echoix commented Sep 10, 2024

nvuillam commented Sep 10, 2024

AlejandroSuero commented Sep 10, 2024

echoix commented Sep 10, 2024

	RUN cargo install --branch main --git https://github.com/Kampfkarren/selene selene
	RUN cargo install selene@0.27.1

	install:
	npm:
	- eslint
	- eslint-config-airbnb
	- eslint-config-prettier
	- eslint-config-standard
	- eslint-plugin-import
	- eslint-plugin-jest
	- eslint-plugin-node
	- eslint-plugin-prettier
	- eslint-plugin-promise
	- eslint-plugin-vue
	- "@babel/core"
	- "@babel/eslint-parser"
	- "@microsoft/eslint-formatter-sarif"

	install:
	dockerfile:
	- \|-
	# renovate: datasource=docker depName=mstruebing/editorconfig-checker
	ARG EDITORCONFIG_EDITORCONFIG_CHECKER_VERSION=v3.0.3
	- FROM mstruebing/editorconfig-checker:${EDITORCONFIG_EDITORCONFIG_CHECKER_VERSION} AS editorconfig-checker
	- COPY --link --from=editorconfig-checker /usr/bin/ec /usr/bin/editorconfig-checker

feat(descriptors, lua): add selene support #3978

feat(descriptors, lua): add selene support #3978

Conversation

AlejandroSuero commented Sep 9, 2024

Proposed Changes

Readiness Checklist

Author/Contributor

Reviewing Maintainer

echoix commented Sep 9, 2024

echoix commented Sep 9, 2024 • edited by github-actions bot Loading

github-actions bot commented Sep 9, 2024

echoix commented Sep 9, 2024

AlejandroSuero commented Sep 9, 2024

echoix commented Sep 9, 2024

nvuillam commented Sep 9, 2024

nvuillam commented Sep 9, 2024

AlejandroSuero commented Sep 9, 2024

nvuillam commented Sep 9, 2024

AlejandroSuero commented Sep 9, 2024

AlejandroSuero commented Sep 9, 2024

echoix commented Sep 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlejandroSuero Sep 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

echoix commented Sep 9, 2024 • edited by github-actions bot Loading

echoix commented Sep 9, 2024

echoix commented Sep 10, 2024

nvuillam commented Sep 10, 2024

AlejandroSuero commented Sep 10, 2024

echoix commented Sep 10, 2024

feat(descriptors, lua): add `selene` support #3978

feat(descriptors, lua): add `selene` support #3978

echoix commented Sep 9, 2024 •

edited by github-actions bot

Loading

echoix commented Sep 9, 2024 •

edited

Loading

AlejandroSuero Sep 9, 2024 •

edited

Loading

echoix commented Sep 9, 2024 •

edited by github-actions bot

Loading