Skip to content

fix: [Concept] String Functions - add exercise templates - #173

Open
webbrain-one wants to merge 1 commit into
zhravan:mainfrom
webbrain-one:webbrain/issue-77
Open

fix: [Concept] String Functions - add exercise templates#173
webbrain-one wants to merge 1 commit into
zhravan:mainfrom
webbrain-one:webbrain/issue-77

Conversation

@webbrain-one

@webbrain-one webbrain-one commented Aug 7, 2026

Copy link
Copy Markdown

Closes #77

Patch generated by qwen3.6-35b-a3b via local API.

Summary by CodeRabbit

  • New Features

    • Added a new “String Functions” exercise covering case conversion, substring replacement, text matching, splitting, and joining.
    • Included starter helpers for practicing common string operations.
  • Tests

    • Added coverage for all supported string manipulation and comparison exercises.

Add templates and tests for the String Functions concept.
Register the exercise in the catalog and include helpful hints.

Closes zhravan#77

Patch generated by qwen3.6-35b-a3b via local API.
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The PR adds a String Functions exercise catalog entry, eight exported string helper stubs, and tests for case conversion, replacement, matching, splitting, and joining.

Changes

String Functions exercise

Layer / File(s) Summary
Exercise definition and function API
internal/exercises/Catalog/Concepts/03_string_functions.yaml, internal/exercises/templates/03_string_functions/strings.go
The catalog defines the exercise metadata and hints. The template declares eight exported string helper functions with placeholder implementations.
String function behavior tests
internal/exercises/templates/03_string_functions/strings_test.go
Tests cover case conversion, substring replacement, containment, prefix and suffix checks, splitting, and joining.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

I’m a rabbit with strings in my den,
Testing them once, then testing again.
Lowercase and join,
Split with great joy,
Eight little helpers await their turn.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the String Functions concept and the addition of exercise templates.
Linked Issues check ✅ Passed The PR adds the requested templates, tests, catalog registration, and helpful hints for issue [#77].
Out of Scope Changes check ✅ Passed All changes support the String Functions exercise template and catalog requirements in issue [#77].
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Warning

⚠️ This pull request shows signs of AI-generated slop (redundant_comments, trivial_assertion). It has been flagged by CodeRabbit slop detection and should be reviewed carefully.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/exercises/Catalog/Concepts/03_string_functions.yaml`:
- Line 8: Update the hint in the string-functions exercise to qualify all checks
with the strings package: use strings.Contains(), strings.HasPrefix(), and
strings.HasSuffix(), avoiding the package-local HasPrefix and HasSuffix
functions.

In `@internal/exercises/templates/03_string_functions/strings_test.go`:
- Around line 52-58: Update TestSplit to compare result against expected
element-by-element using the project’s established full-slice comparison
approach, rather than checking only len(result). Preserve the existing expected
values and Split("a,b,c", ",") input.

In `@internal/exercises/templates/03_string_functions/strings.go`:
- Line 3: Remove the unused "strings" import from the strings_funcs placeholder
in the template so the package compiles while preserving the existing stub
bodies.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1da19633-86d1-469d-8d30-628f55dc0e33

📥 Commits

Reviewing files that changed from the base of the PR and between 6935fea and 2157cba.

📒 Files selected for processing (3)
  • internal/exercises/Catalog/Concepts/03_string_functions.yaml
  • internal/exercises/templates/03_string_functions/strings.go
  • internal/exercises/templates/03_string_functions/strings_test.go

- Use strings.ToLower() for lowercase
- Use strings.ToUpper() for uppercase
- Use strings.ReplaceAll() to replace substrings
- Use strings.Contains(), HasPrefix(), HasSuffix() for checks

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Files matching exercise yaml and related Go files:"
fd -a '03_string_functions.yaml|string|catalog|concepts' . 2>/dev/null | sed 's#^\./##' | head -200

echo
echo "Target YAML content:"
if [ -f internal/exercises/Catalog/Concepts/03_string_functions.yaml ]; then
  cat -n internal/exercises/Catalog/Concepts/03_string_functions.yaml
fi

echo
echo "Find HasPrefix/HasSuffix definitions/usages in repository:"
rg -n --hidden --glob '!*.git/*' 'func\s+Has(Prefix|Suffix)|strings\.(Contains|HasPrefix|HasSuffix)|Has(Prefix|Suffix)' . 2>/dev/null | head -300

echo
echo "Check if strings alias is imported in target exercise implementation files:"
rg -n --hidden --glob '!*.git/*' 'import \(\s*(strings)|"strings"|strings\.|Has(Prefix|Suffix)\s*\(' internal/exercises/Catalog/Concepts . 2>/dev/null | head -300

echo
echo "List files under target directory:"
find internal/exercises/Catalog/Concepts -maxdepth 2 -type f 2>/dev/null | sort

Repository: zhravan/golearn

Length of output: 18351


Qualify HasPrefix and HasSuffix with strings.

The template defines HasPrefix and HasSuffix in the same package and tests them directly, so the unqualified names in the hint can resolve to these exercise functions. Use strings.Contains(), strings.HasPrefix(), and strings.HasSuffix() instead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/exercises/Catalog/Concepts/03_string_functions.yaml` at line 8,
Update the hint in the string-functions exercise to qualify all checks with the
strings package: use strings.Contains(), strings.HasPrefix(), and
strings.HasSuffix(), avoiding the package-local HasPrefix and HasSuffix
functions.

Comment on lines +52 to +58
func TestSplit(t *testing.T) {
expected := []string{"a", "b", "c"}
result := Split("a,b,c", ",")
if len(result) != len(expected) {
t.Errorf("expected %v, got %v", expected, result)
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Files:"
git ls-files | rg 'strings_test\.go|strings\.(go|md|txt)' || true

echo
echo "Test file excerpt:"
file=$(git ls-files | rg '(^|/)strings_test\.go$' | head -n 1 || true)
if [ -n "${file:-}" ]; then
  wc -l "$file"
  sed -n '1,110p' "$file" | cat -n
fi

echo
echo "Implementation candidates:"
for f in $(git ls-files | rg '(^|/)strings\..*$' | head -n 5); do
  echo "--- $f"
  wc -l "$f"
  rg -n "func Split|Split\\(" "$f" || true
done

echo
echo "Behavior probe for current assertion shape:"
python3 - <<'PY'
expected = ["a", "b", "c"]
cases = [
    ["a", "b", "c"],
    ["c", "b", "a"],
    ["x", "y", "z"],
    ["a", "b"],
    ["a", "b", "c", "d"],
]
for result in cases:
    print(result, "current_test_passes=", len(result) == len(expected), "deep_equal=", result == expected)
PY

Repository: zhravan/golearn

Length of output: 2716


Assert the split elements, not only the slice length.

A length check lets any three-element slice pass. Use a full-slice comparison so malformed split results fail.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/exercises/templates/03_string_functions/strings_test.go` around
lines 52 - 58, Update TestSplit to compare result against expected
element-by-element using the project’s established full-slice comparison
approach, rather than checking only len(result). Preserve the existing expected
values and Split("a,b,c", ",") input.

@@ -0,0 +1,51 @@
package strings_funcs

import "strings"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== file location and content =="
if [ -f internal/exercises/templates/03_string_functions/strings.go ]; then
  cat -n internal/exercises/templates/03_string_functions/strings.go
else
  echo "missing file"
fi

echo
echo "== go.mod files =="
fd -a 'go.mod$|go.sum$' . | sed 's#^\./##' | while read -r f; do
  echo "--- ${f#"$PWD/"}"
  sed -n '1,120p' "$f"
done

echo
echo "== lint/config files =="
fd -a 'golangci.*|\.golangci\.ya?ml|\.golangci\.yaml$' . | sed "s#^\./##" | sort | while read -r f; do
  echo "--- ${f#"$PWD/"}"
  sed -n '1,220p' "$f"
done

echo
echo "== package declarations and imports in templates =="
rg -n '^(package |import |import \()' internal/exercises/templates -g '*.go' | head -200

echo
echo "== typecheck/lint config references for templates =="
rg -n 'exercises/templates|templates|golangci|typecheck|unused|enable-all|enable:' . -g '*.json' -g '*.yaml' -g '*.yml' -g '*.toml' -g '*.md' -g '*.go' | head -300

Repository: zhravan/golearn

Length of output: 22132


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== docs verify/incomplete behavior mentions =="
cat -n docs/contributing.md | sed -n '50,75p'
echo "---"
cat -n docs/getting-started.md | sed -n '50,110p'
echo "---"
cat -n CONTRIBUTING.md | sed -n '45,65p'

echo
echo "== CLI verify implementation candidates =="
rg -n 'func .*verify|Verify|verify|template|go run|mod|build|lint|typecheck' internal -g '*.go' | head -250

echo
echo "== inspect relevant CLI file sections =="
ast-grep outline internal/cli/commands.go --match 'func .*Verify' --view expanded || true
sed -n '340,470p' internal/cli/commands.go

Repository: zhravan/golearn

Length of output: 14173


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== internal/cli/commands.go relevant implementation =="
sed -n '120,225p' internal/cli/commands.go

echo
echo "== internal/exercises/solutions.go relevant implementation =="
sed -n '20,85p' internal/exercises/solutions.go

echo
echo "== Go workspace/package references =="
fd -a 'go.work$|go.work.sum$' . | sed 's#^\./##' | while read -r f; do
  echo "--- ${f#"$PWD/"}"
  cat "$f"
done

Repository: zhravan/golearn

Length of output: 5601


Fix the placeholder so it type checks without strings.

golearn verify runs go test ./exercises/03_string_functions, not the embedded internal/exercises/templates package. That package must still compile, but strings_funcs imports "strings" while the placeholder bodies do not use it. If this stub is intended to compile, remove the unused import; do not leave an intentionally broken template unless verification explicitly relies on it.

🧰 Tools
🪛 golangci-lint (2.12.2)

[error] 3-3: : # github.com/zhravan/golearn/internal/exercises/templates/03_string_functions [github.com/zhravan/golearn/internal/exercises/templates/03_string_functions.test]
internal/exercises/templates/03_string_functions/strings.go:3:8: "strings" imported and not used

(typecheck)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/exercises/templates/03_string_functions/strings.go` at line 3,
Remove the unused "strings" import from the strings_funcs placeholder in the
template so the package compiles while preserving the existing stub bodies.

Source: Linters/SAST tools

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Concept] String Functions - add exercise templates

1 participant