#glinter - A configurable linter for the Gleam programming language.

1 messages · Page 1 of 1 (latest)

sonic wolf
#

https://hexdocs.pm/glinter/

A linter for the Gleam programming language. It parses Gleam source files into ASTs using glance and checks them against a configurable set of rules. Many rules are based on the official Gleam conventions.

glinter · v2.16.0

A linter for the Gleam programming language

atomic quiver
#

Docs are bit hard to read!

image0.jpg
#

Also in Gleam config goes in gleam.toml under the tools key, we don’t use additional config files

#

You’re gunna need to change the check interface so that it can hold state like a fold function. You won’t be able to implement many lints otherwise

#

You’ll probably also want to be able to do multiple passes over some of the AST too, and for others stop traversal early to avoid wasted work

#

Linters are challenging to make as they are typically quite expensive to run and the trivial approaches make them painfully slow

#

The quality of the output is worth putting a lot of work into too

#

Elm review is a very nicely designed linter, I would look at that

#

The readme says that you should not add it as a dep to your project- so how come it’s uploaded to hex as a library?

#

And lastly, the big trap I see with linters is that lints get added because they are easy to implement, instead of because they are valuable

#

I would spend as much time researching the pain points as possible before adding a lint

#

A lint that could be really useful today would be one that identifies use of private Gleam data API usage in JS FFI

#

Identifying dynamically building up SQL strings could be another

short rain
#

a lot of value in basic security linters like the SQL one

atomic quiver
#

Yeah I think that would be a key place to focus

eager epoch
#

I'd like one for missing type annotations of public functions

atomic quiver
#

Nothing special about public functions. Private ones should be annotated too

sonic wolf
#

thanks for the feedback! I'll review in a bit

sonic wolf
#

in terms of speed: Linted 103 files (55,762 lines) in 246ms Compiled in 0.02s

#

10x compile speed, but probably still ok?

sonic wolf
#

A lint that could be really useful today would be one that identifies use of private Gleam data API usage in JS FFI
Identifying dynamically building up SQL strings could be another
These totally make sense, but isn't currently useful for our project (we aren't doing JS FFI yet and we use Parrot for SQL). I think this is one I'd ask for a PR on (I'm also happy to pass the flame if someone wants to own this, I'm not exactly the maintainer type! ;))

#

I've addressed all of these other points though. Markdown table on phone (now a list), gleam.toml, and the missing type annotations lint. Fastest v2 ever!

#

I agree that SQL injection checks would be really nice in general, but useless for me right now (and would require a different approach). Might be something that SQL libraries do themselves though? I think sanitizing SQL would be a good, and maybe already assumed, feature of any SQL library

atomic quiver
atomic quiver
sonic wolf
#

i haven't used the gleam sql libraries. i assume they were ORMs? That you have a sanitize and a way to pass args that are safe ala ActiveRecord?

atomic quiver
#

No, there’s no ORMs in Gleam. It’s not possible to implement active record without OOP

#

You’re using typical Gleam
SQL with parrot and sqlight

sonic wolf
#

a function that sanitizes? takes args? ("SELECT * FROM thing WHERE name LIKE ?", arg)

atomic quiver
#

That’s dangerous, you should use the database’s query parameters functionality

sonic wolf
#

i think i'm missing what you're telling me 🙂

atomic quiver
#

Parrot does this all for you, but SQLight is possible to misuse

atomic quiver
sonic wolf
#

yeah, i just don't know how other people are using it i guess. i was just giving an example of how a library might?

atomic quiver
#

A library might what?

sonic wolf
#

use sql

#

pass sql

atomic quiver
#

Sorry I don’t know what you mean.

sonic wolf
#

how a non-parrot or squirrel approach might currently look

#

i guess cake is one?

#

I haven't looked at the API at all for anything else is what I'm saying. I was assuming they'd have sanitation

atomic quiver
#

Yes but the norm is regular database clients

#

No, sanitisation is a mistake

#

Database libraries should not do sanitisation

#

They use the database query parameter functionality as that is safe

sonic wolf
#

i'll take your word for it. i'm honestly only interested in parrot / squirrel when it comes to gleam db access

#

gotcha

#

so what did you want the linter to check for? I think the issue here is me trying to address a concern I don't have (yet).

#

i guess I could see a case where we need to drop out of parrot for an edge case or something

atomic quiver
#

As it’s a package you’ve published to hex for others to use I was saying what could be the most valuable thing for it to have

sonic wolf
#

yes I get it

#

I was actually worried about whether I should publish

atomic quiver
#

Fab fab

sonic wolf
#

I thought it was useful, so i'd share is all

atomic quiver
#

I’m a bit confused as to why it’s in hex as the docs say not to pull it from hex

sonic wolf
#

but I don't want to enshitiffy the community

#

i fixed that

atomic quiver
#

Fab

sonic wolf
#

sorry genx here. does that mean fabulous? lol

atomic quiver
#

Aye! Sorry my British is showing 😁

sonic wolf
#

no worries

#

i'm putting on a tuque before i head downtown today

atomic quiver
#

If it’s on hex folks could perhaps implement their own lints and pass them to glinter.run or something

#

As they can import the code

sonic wolf
#

totally. I'm too new to gleam to really know what's missing. I did some additional speculative rules but discovered they were a bad idea (I was checking for large tuples, but that actually doesn't seem like a code smell in gleam).

#

what's in there right now did catch a bunch of stuff in my codebase that I didn't realize I had missed

#

I suspect I'll find a way to make it faster though. Might even add a parallel option (not reason I can think of it couldn't work) or rework it.

#

not painful enough yet. we'll see

#

what kind of file / line count do the large gleam projects have that you're aware of btw?

distant otter
#

iirc, the biggest Gleam project that was open-source had like 30k lines of code

worldly linden
sonic wolf
#

i'm at 43k right now (excluding generated and tests)

#

@worldly linden that looks like a great test! let me see

# 
  ⎿  Error: Exit code 1
        Compiled in 0.05s
         Running glinter.main
     /tmp/unscii_16_full.gleam:1: [warning] unused_exports: Public constant 'width' is never used by another module
     /tmp/unscii_16_full.gleam:3: [warning] unused_exports: Public constant 'height' is never used by another module
     /tmp/unscii_16_full.gleam:5: [warning] unused_exports: Public constant 'default_glyph' is never used by another module
     /tmp/unscii_16_full.gleam:7: [warning] unused_exports: Public function 'render_glyph' is never used by another module

     Found 4 issues (0 errors, 4 warnings)
     Linted 1 file (21,108 lines) in 1478ms```
worldly linden
#

Nice

sonic wolf
#

bit slow i'd say

worldly linden
#

That one has a monster BitArray pattern

sonic wolf
#

failed to parse

#

glance doesn't like it

#

maybe we need a glance update for that one

sonic wolf
#

made a bunch of updates. on v2.2.0 now

atomic quiver
#

After that can identify common state that multiple lints collect and extract them into an earlier pass that multiple lints then use the data from

late ridge
#

It’s a client database for posgress where you write your SQL query (as a string literal preferably) than pass parameters for the SQL placeholder and lastly a decoder so you can have a Result with the row decoder as a gleam type

# 
import pog
import gleam/dynamic/decode

pub fn run(db: pog.Connection) {
  // An SQL statement to run. It takes one int as a parameter
  let sql_query = "
  select
    name, age, colour, friends
  from
    cats
  where
    id = $1"

  // This is the decoder for the value returned by the query
  let row_decoder = {
    use name <- decode.field(0, decode.string)
    use age <- decode.field(1, decode.int)
    use colour <- decode.field(2, decode.string)
    use friends <- decode.field(3, decode.list(decode.string))
    decode.success(#(name, age, colour, friends))
  }

  // Run the query against the PostgreSQL database
  // The int `1` is given as a parameter
  let assert Ok(data) =
    pog.query(sql_query)
    |> pog.parameter(pog.int(1))
    |> pog.returning(row_decoder)
    |> pog.execute(db)

  // And then do something with the returned results
  assert data.count == 2
  assert data.rows == [#("Nubi", 3, "black", ["Al", "Cutlass"])])
}
sonic wolf
#

Linted 172 files (65,583 lines) in 69ms

#

more like 6x i guess. still

atomic quiver
#

Awesome

sonic wolf
#

need to pay the bills for a bit, but I might tackle your SQL suggestion later.

sonic wolf
#

@atomic quiver do you have an opinion, or do you know if there's acommunity opinion / consensus on module complexity (total case/fn/block expressions)? I've included it, but I'm tempted to default it to off because I don't personally think module size matters... but I'm probably an outlier on that.

late ridge
#

Please don’t force people to have small module

#

Having large module is great

sonic wolf
#

yes exactly. i have elm modules that around thousands of lines

#

i'm going to default it to off then

late ridge
#

Also I’m not sure if having a cyclomatic complexity linter make sense in functional programming

#

Function are naturally simple and do what they need

atomic quiver
#

I don't think complexity is a good thing for a linter. I've not seen any complexity metric that always agreed with human reviewers and also works well for FP

#

Module complexity specifically I think is nonsense

#

Functions in a module don't impact each other, so counting their complexity as one makes no sense to me

#

You don't need to hold the other functions in your head to read one function in the same module

#

Complexity I think it better used interactively, for help with refactoring decisions

#

"of the most used functions with the highest git history churn rate, which have the highest complexity" etc

sonic wolf
#

i agree. just looking at what the linter has caught for us, the complexity checks were kind of useless. deep nesting I think was useful though

late ridge
sonic wolf
#

yeah, so the deep nesting check is useful to catch that

sonic wolf
#

v2.6.0 adds JS FFI and SQL concat checks. Dunno how useful they are but I figured they would be a good starting point for feedback or a PR.

#

default to off

trail hedge
#

how do they work??

atomic quiver
#

the SQL concat one you'd need to implement an effects tracking system I wager

#

mm it doesn't actually do what it says it does

#

Rather it's a lint that bans any use of <> with string literals that contain any of these words:

const sql_keywords = [
  "select ", "insert ", "update ", "delete ", "drop ", "alter ", "create ",
  " where ", " join ", " from ",
]
#

That's going to result in a lot of false positives, and it won't catch actual dynamic SQL

late ridge
#

Hopelly with postgress I saw I could use postgre array to not have to deal with IN

atomic quiver
#

You can use any for that

#

SQL has features for most things people use string concat for

late ridge
#

(and yeah I discovered it and I love it)

sonic wolf
atomic quiver
#

I think it would be best not to publish rules that don't work and if turned on will cause problems

sonic wolf
#

agreed. actually removing it now

#

sql check is gone. tbh an effects tracking system is pretty big and probably not in the cards for me, at least not in the near term.

sonic wolf
#

I feel like glinter is in a pretty good place now. It doesn't do SQL concat checks (that a whole big deal), but it has a collection of usefull checks that I'm benefitting from in a decent sized codebase. Any opinions on having the ability to annotate functions to make them exempt? Right now it's all in toml without function granularity, so it would be nice to say "don't run these 4 lints on this function", but also annotations pollute code so... maybe not keen for that reason.

trail hedge
#

why would you want the ability to opt out

sonic wolf
#

because some rules are a bit too picky for some functions

#

see this the most in tests (public functionas)

trail hedge
#

sounds like a bad rule to me ^.^

#

linters with per-function or per-expression opt out almost always devolve into a codebase of exceptions and "i'll disable that and come back to it later"s

sonic wolf
#

i had the same thought inititally, but then how do you capture a code smell that is usually but not always a code smell

#

let me see if I can give you a concrete example

#

where it's not always black and white

trail hedge
#

@jagged lagoon will have lots to say on this topic (:

sonic wolf
#

dunno i think if it was always 100% perfect you'd probably just put it in the compiler / formatter

#

because why support the non-case if it's never right

#

whereas a linter needs a bit more flexibility to be "hey, this doesn't look good, and 99% it isn't, but maybe you have a good reason"

#

one example, although not at a function but a glob level, is my generated db code

#

my tests are another

trail hedge
#

neither of those seem like function granularity which is what you asked about!

sonic wolf
#

yes i'm still looking. i know i ran into it a couple times

trail hedge
#

in my experience linters that aim for this flexibility end up loathed and ignored. because someone's patience for a linter that flags false positives wears quickly and usually one of two things happen:

  • they remove the rule
  • they develop muscle memory to add ignore comments whenever the linter flags something
sonic wolf
#

I think you'll always have mixed feelings on linters. I do find them usefull myself though as extra guard rails not covered by the compiler.

#

I'm willing to put in the effort to try and make it useful to others, but only so much tbh

#

anyways, i have some examples. sec

#

let assert Ok(secret) = envoy.get("SESSION_SECRET")

#

The lint rule correctly flags every let assert Ok because 99% of the time you should handle the error.

#

but in this case, crashing is correct

#

this is in my pub fn start()

trail hedge
#

aye, in my opinion this is a bad rule ^.^ there are plenty of scenarios where assertions and panics are the correct thing to do and it extends beyond reading the environment. crashing is a fundamental part of how BEAM applications are architected and so this lint will report false positives many times in a legitimate program

#

this trains the developer to simply ignore the lint or to remove it

sonic wolf
#

ok, but for me this is useful almost everywhere else

#

see what i mean?

#

I 100% want this rule

#

I just want to ignore it in very very few cases (basically just here and in generated code)

#

so right now, i ignore this rule for the file

#

not a huge deal, but ignoring it for the function would be better for me

#

definitely not better enough to introduce annotation noise though, but I have some others

#

string_inspect, thrown_away_error, and maybe deep_nesting are others

trail hedge
#

a better rule would be configurable at the rule level not at the callsite, for example: "no let assert except for envoy.get" is a thing that would be totally expressible as config

#

and directly encodes the practices you want to enforce

sonic wolf
#

true. that's worth looking into

#

i used this example, because this function has more than one

#

let assert Ok(db_path) = envoy.get("CURLING_DB_PATH")

#

but yeah, same deal

#

although, if i did that in a request handler instead of start

#

i wouldn't do that though...

#

for other reasons

#

what i'm trying to say at the function level though is that "this function is allowed to crash"

trail hedge
#

it's often the case that linters are simply not expressive enough to codify the practices we want to enforce, and its a big reason why ad-hoc opt-outs are possible, because the linter is not good enough. no reason we have to have this problem though, instead of adopting bad practices from other linters we could just make sure it's possible (and ideally easy) to write rules that precisely describe the best-practices.

not just "this function right here is allowed to crash" but rather why is it allowed to crash and how do we codify that in a rule so we don't have this ad-hoc ignore but instead an enforced practice

sonic wolf
#

I don't think there's enough information in the code for the linter to know

#

an annotation does that... but tbh I don't love annotations

#

basically i'm going to write code that crashes and forget about it, but realisticaly I'd prefer it didn't in almost all cases

#

the linter will find that code, say "hey man, you sure you wanted this to crash?"

#

I'm like nope, let me fix that

#

that being said, i do agree. it would be awesome if the linter knows when crashing is cool. E.g. "this runs under supervision and failure means we're cooked, so crash is cool."

atomic quiver
#

I agree with Hayleigh here. The challenge of a linter is designing the rules to be net positive for productivity

#

We don't want to fall into the hole that the JS and Ruby folks have with lots of lints where teams fight them and spends their time doing busy-work changes and adding comments to turn things off

atomic quiver
sonic wolf
#

hmmm. i think I might need a new design for this

atomic quiver
#

I think studying productive linters such as elm-review and sobelow would be a great first step when making a linter

sonic wolf
#

scope creep. this started as a basic bash script I was using to catch basic stuff

#

ok I 100% agree with you both. It's definitely a bigger task. I'm going to look into it though. specifically the elm-review philosophy and how close I can get to that with gleam. issue seems to be getting type info, but i'll try a few things.

trail hedge
#

fwiw elm-review went a realllllly long time before they added type inference and still worked great

#

also he writes loads 😂

sonic wolf
#

awesome

jagged lagoon
#

@sonic wolf I'm happy to answer questions you have about linting and/or elm-review. The talk linked just above is a good place to start, otherwise the docs are wordy but a good place to learn about some philosophies too. https://elm-review.com/

sonic wolf
#

i've used elm-review actually, but never dug into the details

#

i'm working on a first pass now. we'll see how it goes. love that you're here so I can ping you about it

trail hedge
#

jeroen will talk about linters to literally anyone that will listen

jagged lagoon
#

As the others pointed out as well, a linter IMO should not report code smells, it should report bad patterns. Simple lint rules are easy to easy to make and therefore attractive, but where a great linter really shines is when it only reports true positives. Remove false positives as much as possible, and then try to reduce the false positives to a maximum.

For instance, the let assert example above is a valid and worthwhile piece of code, so it would be great to figure out in which cases it's definitely bad, and try to specifically report those. That might mean you need to gather a lot more information about surrounding code, and sometimes it's just plain not possible.

jagged lagoon
sonic wolf
#

yes i'm actually reviewing elm-review code right now and looking to create a complete rework modeled after that, if I can

#

like phantom types

#

think to start with i'll skip that 🙂

jagged lagoon
#

elm-review is basically roughly what you'll end up If you remove the warning level (only have error) and remove suppression comments. It will suck unless the rules are really good 😄

#

I ported a starting version of elm-review to Gleam. I think the repo was renamed to code-review, but I don't remember under which GH user

sonic wolf
#

oh i did see that. from back in 2024. I didn't realize it was you

trail hedge
#

it was my attempt at coaxing him to swap teams

jagged lagoon
#

I'll say it again, I'll gladly use Gleam the day I use it at work. Btw I'm in the process of quitting my job right now 😄

sonic wolf
#

Jeroen, you joined discord 4 days before I did

jagged lagoon
#

Call me senpai

#

Gleam has a fairly small number of language syntax, so you're very unlikely to produce any good rule by outright forbidding any piece of syntax (like let assert). I'm sure the core team would have removed that feature already if it was always bad.

Where I see value in a linter for Gleam, off the top of my head:

  • Customizable rules: Being able to create new guarantees that make sense for a codebase (or the use of a specific framework) that don't make sense for general Gleam
  • Auto-fixable errors for some of the existing reported errors (compiler warnings IIRC?), but that does mean errors will be reported twice.
  • Errors for problems that are valid Gleam code but that we don't want. Ex: unused code. (unless I missed things, I don't think the compiler catches everything, because some are really time-consuming)
  • And at the very bottom, code style issues that are not already handled by the formatter. There's value in making things consistent, but there are more valuable things to reach out to first.
#

Where Gleam differs from elm-review and that will make things a bit harder to figure out: there's side-effects, and you don't have total knowledge of the codebase (some functions will be used in Erlang/JS and you therefore can't consider it unused)

atomic quiver
#

Customizable rules are awesome. "The database module should not be usable within the ui directory" etc

late ridge
sonic wolf
#

ok 2.11.0 is out. visitor pattern lke @jagged lagoon used, giving us more contextual info. Also improved some rules, added some more, and setup the plugin system. My lints are probably too much for most, so let me know if you'd like any of these to default to off. I think having core rules ship with it is more intuitive for users than requiring them to grab plugins, but open to being convinced otherwise. https://hexdocs.pm/glinter/

glinter · v2.11.0

A linter for the Gleam programming language

#

Linted 191 files (78,917 lines) in 1540ms which is about 40% slower than before, but seems acceptable for the recursive traversal we needed

#

oooh, cached the parsing, one minor tweak and we're 20% faster than before: Linted 191 files (78,917 lines) in 925ms

jade swan
#

Hey, just discovered the package, linter is so cool. To create custom linting rules in my project, I need to write some configuration code in Gleam? For instance, if I want to implement Redraw linting, I'd need every user to write a bit of Gleam to implement the linter? 🙂

sonic wolf
late ridge
#

I have a idea of linter, warn when you are using marked as internal module in your project by accident 😅

sonic wolf
#

2.16 is out. it has @internal awareness. I was using this in a project to make some private methods public to be testable. Linter will ignore uncalled public functions marked with internal annotation and it will flag private functions marked with internal (leftovers maybe?)

soft granite
#

Private functions with the internal attribute are already a compiler error 😁

sonic wolf
#

how do you use @interal @soft granite ? I was looking for a way to expose a private function for better test coverage and that's how I found it, but... not sure it's the intent? All I could find was that it was mainly meant to just mark something as excluded from the docs and warn if called

trail hedge
#

internal as in "not part of the public (aka documented) api" not internal as in "not accessible outside this package"

sonic wolf
trail hedge
#

depends really, sometimes unit testing gnarly internals is necessary, sometimes just testing the public interface would be enough