Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve remaining Lint cops #9180

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Commits on Dec 6, 2023

  1. Lint/ConstantDefinitionInBlock (metaprogramming)

    When metaprogramming types, parameters and providers, we defined constants
    within the block, such as
    
        Puppet::Type.type(:package).provide(:xxx) do
          SOMETHING = ...
    
    The constant isn't namespaced in the way you'd expect and instead is defined at
    top-level, potentially clashing with non-Puppet code. In order to define
    constants on the generated class:
    
       Puppet::Type::Package::ProviderXXX
    
    use `const_set` instead. But continue providing the "legacy" constant to
    maintain backwards compatibility and mark it as deprecated. A deprecation
    warning will only be generated if the legacy constant is accessed when running
    ruby with -W2:
    
        $ bundle exec ruby -W2 -rpuppet -e "Puppet::Type.type(:package).provider(:yum); RPM_VERSION"
        ...
        -e:1: warning: constant ::RPM_VERSION is deprecated
    joshcooper committed Dec 6, 2023
    Configuration menu
    Copy the full SHA
    ed47d1e View commit details
    Browse the repository at this point in the history
  2. Lint/ConstantDefinitionInBlock (faces)

    Constants defined in blocks aren't namespaced and instead are created in the
    top-level namespace, e.g. DEFAULT_SECTION.
    
    Define a new module for each face in which to store their respective constants
    and deprecate the old top-level constants. A deprecation warning will only be
    generated if the legacy constant is accessed when running ruby with -W2.
    joshcooper committed Dec 6, 2023
    Configuration menu
    Copy the full SHA
    a12bba4 View commit details
    Browse the repository at this point in the history
  3. Lint/MissingSuper (NEEDS WORK)

    Explicitly call `super` to avoid bugs that can occur if functionality is added
    to a superclass' initialize method, but isn't added to all of the subclasses
    that don't call `super`.
    
    There are a few places where it's not clear if calling super is the correct
    thing, and in those cases the cop has been disabled.
    
    In cases where the super and subclass initialize an instance variable, have the
    subclass call to the superclass' initialize method, such as the
    EvaluatingEppParser/EvaluatingParser, PSensitiveType/PAnyType, and HTTP client
    resolvers.
    
    Also call super for `inherited` and `method_added` lifecycle methods.
    joshcooper committed Dec 6, 2023
    Configuration menu
    Copy the full SHA
    bb21107 View commit details
    Browse the repository at this point in the history
  4. Lint/ToJSON (NEEDS WORK)

    Calling `JSON.generate(obj, {})` on an object will have its `to_json` method
    called and the options has will be passed as an argument. If the object doesn't
    accept the argument, then it will result in:
    
        ArgumentError (wrong number of arguments (given 1, expected 0))
    joshcooper committed Dec 6, 2023
    Configuration menu
    Copy the full SHA
    3b981fe View commit details
    Browse the repository at this point in the history
  5. Lint/UnusedMethodArgument

    This cop flags methods where `&block` is passed to a method, which is a very
    common practice. There's also debate[1] as to whether specifying `&block`
    explicitly is better than implicitly accepting a block.
    
    [1] rubocop/rubocop-performance#385
    joshcooper committed Dec 6, 2023
    Configuration menu
    Copy the full SHA
    b9b9076 View commit details
    Browse the repository at this point in the history
  6. Lint/SuppressedException

    StopIteration is used for flow control.
    
    Ignore scripts in util directory.
    
    Rescuing LoadError for CreateSymbolicLinkW is left over from 2003, which is long
    since EOL.
    
    If we get Errno::EAGAIN, then use `retry` to make it explicit.
    joshcooper committed Dec 6, 2023
    Configuration menu
    Copy the full SHA
    db6c729 View commit details
    Browse the repository at this point in the history
  7. Lint/RescueException

    Rescuing Exception is generally not recommended, because it will also rescue
    SystemExit and SystemStackError (and more), preventing an application from
    gracefully exiting.
    
    Rescuing Exception is sometimes needed when calling 3rd party code, because that
    code might raise an exception that doesn't inherit from StandardError. For
    example, autoloading a provider might raise a LoadError if the provider relies
    on a missing gem. Since LoadError extends ScriptError which extends Exception,
    it is not caught when doing `rescue => e` since that only rescues StandardErrors.
    
    In cases where we rescue Exception and immediately re-raise, then ignore the
    cop, since a SystemExit will cause us to exit.
    
    If we rescue the Exception, but try to continue along, then first rescue
    SystemExit and re-raise that.
    
    The Windows daemon code is fragile, so just ignore the cop there.
    joshcooper committed Dec 6, 2023
    Configuration menu
    Copy the full SHA
    0ca2680 View commit details
    Browse the repository at this point in the history
  8. Configuration menu
    Copy the full SHA
    0cd3012 View commit details
    Browse the repository at this point in the history