Shopify / Shopify/liquid

Fix Utils module to not expect arguments for inspect

Open Beginner friendly
#2,107 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Ruby
Stars
11.9k
Forks
1.5k
Avg merge
17h 55m
Merged PRs (30d)
3

Description

Why

Hello. 👋

The Liquid::Utils.inspect method expects an object argument but, when not given, it'll throw an ArgumentError. This violates the Liskov Substitution Principle becauseObject (superclass) does not require you pass in any arguments.

How

To fix, can the method signature be something like this?

self.inspect object = nil, seen = {}

This would allow object to be optional instead of requried.

Notes

I tried monkey patching with nil but ended up with different spec failures so not sure what the default expectation is here but this definitely causes specs to fail in CI when using SimpleCov 1.0.0. Although, when running locally, I could not recreate this.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at Liquid::Utils.inspect and reproduce the missing-argument failure described in the issue. Read the existing implementation and related specs to determine the expected default behavior, then verify that calling inspect without arguments no longer raises an ArgumentError and that the relevant specs pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
ruby
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
65/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.