Skip to content

Proposal: Change linter to allow globals and disallow mutation of global variables #41

Open

Description

@leighmcculloch

Proposal

Change linter to allow globals and disallow mutation of global variables.

Why

This linter's goal is to reduce side-effects that arise out of global (package level) variables. This is challenging because there are multiple common patterns using package level variables. Many are already exceptions, such as sentinel error values, regular expressions, then there's also a handful of other cases like #33 that aren't currently handled.

Side-effects arise out of the use of globals because any function in a package can mutate an unexported package variable, and any function anywhere in an application can mutate an exported package variable. When any functions mutates these values it can have an effect on other code in the application.

Preventing mutations of package level variables would have the same result of preventing side-effects, since globals that are not mutated do not cause side-effects.

This change would allow globals to be used as pseudo-constants in the many patterns that Go developers use them as such.

This issue is a request for comment from anyone using this linter.

How

It's unclear to me if there's a reasonable way to detect mutations of package level variables.

The simple case of reassignment is, I think, probably easy. We'd need to look for statements that assign to a value where that value being assigned to is a package level value.

However, there's the difficult case of what if someone passes a reference to a package level value to a function. Or what if the type of the package variable is a struct with functions that mutate itself.

If anyone has ideas, please post it to this issue.

Activity

  1. leonklingele commented on Feb 7, 2024

    @leonklingele
    Contributor

    imo, this should be part of curioswitch/go-reassign#18.

  2. leighmcculloch commented on Feb 9, 2024

    @leighmcculloch
    OwnerAuthor

    Interesting, I didn't know about that linter. It looks swish.

    imo, this should be part of curioswitch/go-reassign#18.

    Maybe. It's unclear to me if that would be scope creep for that linter, but I don't know what their plans are. My understanding of the linters goal is to prevent outside mutation of package level variables that are intended to be publicly accessible by not publicly mutable, and essentially add an access modifier to the package level variables that Go doesn't have as a first level feature.

    I think the goal of this issue would be to prevent any mutability of package level variables by anyone, even internals, and the goal is quite different: The goal is to reduce side-effects.

    So the goals of both, assuming I'm understanding the goals of go-reassign correctly – which I might not be, are I think different.

  3. rostislaved commented on Apr 10, 2025

    @rostislaved

    Hi @leighmcculloch
    having go-reassign linter maybe consider adding setting to exclude some cases? We need to be able to exclude time.Date, regex.Must etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions