james-d-mitchell / gaplint Goto Github PK

View Code? Open in Web Editor NEW

7.0 4.0 4.0 159 KB

A linter for GAP (https://www.gap-system.org/)

License: GNU General Public License v3.0

Makefile 0.23% Python 78.46% GAP 21.30%

syntax-checker syntax-checking lint linters lint-checking

gaplint's People

Contributors

Stargazers

Watchers

Forkers

simon111111 flsmith cdwensley reiniscirpons

gaplint's Issues

Test failure from release tarball

When using the tagged release from e.g. https://github.com/james-d-mitchell/gaplint/archive/refs/tags/v1.1.5.tar.gz, one test fails because it's looking for .git:

$ pytest
============================= test session starts ==============================
platform linux -- Python 3.11.9, pytest-8.1.1, pluggy-1.4.0
rootdir: /home/mjo/src/gaplint-1.1.5
configfile: pyproject.toml
plugins: pkgcore-0.12.27
collected 22 items                                                             

tests/test_gaplint.py ...................F..                             [100%]

=================================== FAILURES ===================================
_____________ TestConfigYAMLFile.test_with_config_file_parent_root _____________

self = <tests.test_gaplint.TestConfigYAMLFile testMethod=test_with_config_file_parent_root>

    def test_with_config_file_parent_root(self):
>       os.rename(".git", ".tmp_git")
E       FileNotFoundError: [Errno 2] No such file or directory: '.git' -> '.tmp_git'

tests/test_gaplint.py:245: FileNotFoundError
=========================== short test summary info ============================
FAILED tests/test_gaplint.py::TestConfigYAMLFile::test_with_config_file_parent_root - FileNotFoundError: [Errno 2] No such file or directory: '.git' -> '.tmp_git'
========================= 1 failed, 21 passed in 0.44s =========================

No warnings are issued if bad codes/names are put into a `.gaplint.yml` file.

Suggestions and thoughts for the wiki entries for vim and neovim

Two obvious simple fixes

Fix link for neovim wiki entry on https://github.com/james-d-mitchell/gaplint/wiki/ (not sidebar)
Replace word "syntastic" with "ale" with its hyperlink on https://github.com/james-d-mitchell/gaplint/wiki/Using-gaplint-in-neovim (first line; probably a copy and paste error)

Other points

Mention alternative installation method of gaplint with pipx which has the advantage of using isolated environments avoiding things like https://xkcd.com/1987/
I am not sure why ale is advertised for neovim and syntastic for vim. Ale works for both as far as I know and syntastic has been marked as archived which IMHO makes syntastic obsolete and should not be used by new users.
IMHO using nvim-lint would be more suitable for neovim:
https://github.com/mfussenegger/nvim-lint#custom-linters
I would say one can suggest ale for vim and nvim-lint for neovim
What about creating a vim compiler plugin gaplint.vim? This could even be added to upstream vim and neovim.
https://github.com/vim/vim/tree/master/runtime/compiler There exist already quite a few linter as compiler plugins: eslint, jsonlint, pylint, and more.
```
if exists("current_compiler")
  finish
endif
let current_compiler = "gaplint"

if exists(":CompilerSet") != 2		" older Vim always used :setlocal
  command -nargs=* CompilerSet setlocal <args>
endif

" Runs gaplint for current file :h cmdline-special
CompilerSet makeprg=gaplint\ %

CompilerSet errorformat=%f:%l:%m\ [%n]
" ignore unmatched lines
CompilerSet errorformat+=%-G%m
```
Install into $VIM/compiler/gaplint.vim, activate with :comp[iler] gaplint and run :mak or use vim-dispatch with its mapping m<CR>. For this idea see also: https://gist.github.com/romainl/ce55ce6fdc1659c5fbc0f4224fd6ad29

SetuptoolsDeprecationWarning: Invalid dash-separated options

This is trivial, but newer setuptools complain...

/usr/lib/python3.11/site-packages/setuptools/dist.py:476: SetuptoolsDeprecationWarning: Invalid dash-separated options
!!

        ********************************************************************************
        Usage of dash-separated 'description-file' will not be supported in future
        versions. Please use the underscore name 'description_file' instead.

        By 2024-Sep-26, you need to update your project and remove deprecated calls
        or your builds will no longer be supported.

        See https://setuptools.pypa.io/en/latest/userguide/declarative_config.html for details.
        ********************************************************************************

!!

The offending line is,

https://github.com/james-d-mitchell/gaplint/blob/main/setup.cfg#L4

Suggested gaplint -- do not use `arg`

Traditionally GAP used the special name arg for functions with a variable number of arguments, but now there is ... instead, so check for function with a single argument arg, and suggest using ... instead.

Handling long strings in test files

Chris added the following test to a test file in the Digraphs package:

gap> gr := DigraphFromDiSparse6String(
> ".~?CI_A?WA_M@G@_G@gB?]@?G_SAWA?Y@oJ__BGH?uA_M_IAoO_oCWL@IB_R_{DGB?mB?U_sDwM@a\
> BoX_KCGP@WEwQ@[FGR@_FWS@cFgT@gFwB@A?oO_KCGZACGwZAGGw[AUFOcAYF_f`{IG_Ae@?U`[IwW\
> AqEOl`gJgC@mF?jBAFOkBEF_lBIFomBMG?nBQ@?ZAE@?ZAI@?ZAMH?oBWMGdB?LowaWKOya[K_xBmI\
> ?rBqIOsBuI_tBy@?`AMH?uCA@?aaOHOuB[OgCAIHOvCQ@?baOM@?CYMP@C]HOwCYMOyCKPpHbgPPHb\
> kQw{CgRG|C_RW}CyNpN_SIwkDEJPQawSwnDQ@OobCSPUbGS`VbKSpWbOT@XbSTPY_SK?u_SK?v_SK?\
> wbcT`[DyM`UDsVgzD[WG{D_Vp`bsUPabwU`bb{Upc_SL`?bcOP[EY@OvCIMOyCKV@\\Ea@OvCQM`DD\
> sYgDB_PgxC[V`kc_VpfEuM`HDwZHID{W@hEsZxJE?YpncoWPpcsW`oFIR`bEw[xNEO\\HOES\\WDC?\
> Ph@C[X`vc_Xpw_SOhBEWY@yc_Q`fEc]wDCQH@DE_Y`|cgQphEk^gDCQPPiGAQpjGE@PEc[Z@vGMQ@l\
> F_`HmFc`XHEo_xIEsZqCG]Z`oFo`QGckZqFf?[P~G_ahpGGahqGq[qJGu\\AHGy\\QEG}\\aO_G@gF\
> @?cgGAKcwHCWdGIGKdWA_KCAV`CeGQACeWRAOegSC[ewTGOfG`AGeGBH]GaWH}@?bHaH?dC?Oa]IEH\
> Q_IEHaba[gaca_OQdacQAeag`QfcCOqa_OeA^aCO`CI?ihBCSgabIm@A^cOgAlcSgqm_SPa`c[QPvF\
> ggaoc_Q`wFkiQpccgqocgQqkJCkxJI{kxKJURQsJYRaqJ]RpxJaSAEJe?qQaGcqzaSdA{ccdQ|g[da\
> }hcigQCGoHBHkoXIHooiGHsoy^i?ialKU@Q`IiH@yFsgqjK]grEK]hBHa[oRGKiI@zISobJfohrKgc\
> iBL_OmxCJorxDJssHJJwsYIJ{sgDIgjZ?LQC`|LU^aZKCth~KKtyJKOuGDK}_BOLi_RPLm_bQLqbBR\
> Lu@gPHKvweJswHKLGwYLLwwgDIuCaVMAHqZH{wRdcsjRBMGxiMLcwrcM]GP|G?jbELQ_AmMQjRDkWt\
> BcMmxBja_fa_KSxhMI[iqmKWxrjMybrMM_yRiMozRncC]p}IoqH}GCganMeOamKYOqnK_qRhNMjbEM\
> mO`|MgzBtmgzRti{qRkNUyzkMs|Rxms}XNIcjAnM{{rtNe_QnMg|YOJg{BpNG|BuN[}ByNk~B|fc^A\
> qfo^qqJO{XzNG|H~GGlAtNG|H}K_|bwNu^RvNi_atN_~ZvN_}b|n[}bznmcR}N|?C@OH?sCOT@cFOb\
> AiQ@ECgR`QDWaaSOgea]IGhaiOxCcUQPycg]xJcqRXMc}SH{fsgx}f}_I@gI`yGgeaiJgqbYMg}cIP\
> heeiZhqfYciUhifiajinjMlItjYlywjemj?kEojBkQqZIkmrJLkytZUl]uJXmkyjrNI}Rv");

Gaplint complains WARNING in tst/standard/prop.tst:1077 unmatched quote!.

However, I think that this usage should be allowed.

pytest args are passed through to main()

When using pytest to run the test suite, any additional args passed to pytest wind up in sys.argv, and then argparse treats them as arguments to gaplint. For example,

$ pytest -vv
...
=========================== short test summary info ============================
FAILED tests/test_gaplint.py::TestScript::test_disable_global_rule - AssertionError: 'Aborting!' != 0
FAILED tests/test_gaplint.py::TestScript::test_wrong_ext - SystemExit: Aborting!
FAILED tests/test_gaplint.py::TestRules::test_run_gaplint - SystemExit: Aborting!
========================= 3 failed, 18 passed in 0.72s =========================

Each of those is complaining about the unknown argument -vv:

gaplint.py:1110: SystemExit
----------------------------- Captured stderr call -----------------------------
Unknown command line option: ['-vv']

One potential fix for this is to wipe sys.argv in main() if it was called from... elsewhere:

def main(**kwargs) -> None:
    if __name__ != "__main__":
        sys.argv = []
    args = _parse_args(kwargs)

I ran in to this on Gentoo where the package manager runs pytest with a crazy string of arguments.

Request: more distinctive WARNING lines

In the Travis tests for Digraphs and Semigroups, we run various tests and tools, and then we use grep to search the output of these processes for certain keywords. At the end of its output, Travis displays the output of this grep. Thus, each failed line that it finds is printed. I find this useful.

Currently, when gaplint encounters a problem, it prints something like:

WARNING in gap/attr.gi:46   no space allowed before bracket  [1]

The problem with this is that the obvious failure keyword here is WARNING, and it's doesn't let me discriminate sufficiently. If I have WARNING as one of the keywords, then the Travis tests begin to fail because of innocuous warnings like:

#W WARNING: cannot find 'pdflatex', please install TeX.

It would be great if you update the warning to be something more distinctive, perhaps by including the word gaplint. eg:

gaplint: WARNING in gap/attr.gi:46   no space allowed before bracket  [1]

or something better. It's not a massive issue, since at the end of its search, gaplint prints FAILED if anything went wrong, and that's certainly something I can search for as a failure word. However, it doesn't give us the line-by-line list of every failure that is handy to have repeated at the end of the Travis file.

Add 'enabled' to enable a small set of lints

For GAP core, we only use a small set of lints, and the list of 'disabled' lints has to be extended when new lints are added.

If you could give a list of enabled lints instead, that would mean new releases with new lints wouldn't break CI.