Skip to content

Commit 0686cd6

Browse files
aledbfclaude
andcommitted
refactor(cli): drop checks no outcome depends on
Mutation testing showed these edits change nothing a caller sees: skipJSONValue only meets opening delimiters at a value's position and can consume an object's keys like its values; orderedObjectKeys' empty input already fails the first token; ociAnnotationID's missing or empty annotation already fails to unmarshal; filepath.Localize only returns local paths, so the IsLocal check after it refuses nothing; every caller of downloadFeatureTarball passes a context; and the replacer's capacity hint is unobservable. The 512 MiB download cap is exempt, as only a 512 MiB response could show it move. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent b88b62d commit 0686cd6

3 files changed

Lines changed: 22 additions & 33 deletions

File tree

‎internal/cli/collection_commands.go‎

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,7 @@ func generateDocs(projectFolder, registry, namespace, githubOwner, githubRepo, c
599599
}
600600

601601
func renderDocsTemplate(template string, values map[string]string) string {
602-
pairs := make([]string, 0, len(values)*2)
602+
var pairs []string
603603
for name, value := range values {
604604
pairs = append(pairs, "#{"+name+"}", value)
605605
}
@@ -707,15 +707,9 @@ func defaultCell(ov map[string]any) string {
707707

708708
// orderedObjectKeys returns the keys of a JSON object in source order.
709709
func orderedObjectKeys(raw json.RawMessage) []string {
710-
if len(raw) == 0 {
711-
return nil
712-
}
713710
dec := json.NewDecoder(bytes.NewReader(raw))
714711
t, err := dec.Token()
715-
if err != nil {
716-
return nil
717-
}
718-
if d, ok := t.(json.Delim); !ok || d != '{' {
712+
if err != nil || t != json.Delim('{') {
719713
return nil
720714
}
721715
var keys []string
@@ -734,27 +728,24 @@ func orderedObjectKeys(raw json.RawMessage) []string {
734728
return keys
735729
}
736730

731+
// skipJSONValue consumes the next value from dec. At a value's position the
732+
// decoder only ever yields an opening delimiter, so any Delim starts a
733+
// container; an object's keys are string tokens, consumed like its values.
737734
func skipJSONValue(dec *json.Decoder) error {
738735
t, err := dec.Token()
739736
if err != nil {
740737
return err
741738
}
742-
if d, ok := t.(json.Delim); ok && (d == '{' || d == '[') {
743-
for dec.More() {
744-
if d == '{' {
745-
if _, err := dec.Token(); err != nil { // key
746-
return err
747-
}
748-
}
749-
if err := skipJSONValue(dec); err != nil {
750-
return err
751-
}
752-
}
753-
if _, err := dec.Token(); err != nil { // closing delim
739+
if _, ok := t.(json.Delim); !ok {
740+
return nil
741+
}
742+
for dec.More() {
743+
if err := skipJSONValue(dec); err != nil {
754744
return err
755745
}
756746
}
757-
return nil
747+
_, err = dec.Token() // closing delim
748+
return err
758749
}
759750

760751
// createTarArchive creates a plain (uncompressed) tar archive from a directory,

‎internal/cli/feature_deps.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -163,14 +163,14 @@ func featureMetadataFromTarball(blob []byte) (features.Feature, error) {
163163
return meta, nil
164164
}
165165

166+
// ociAnnotationID is the id in the manifest's metadata annotation, or "" when
167+
// the annotation is missing, empty or not JSON (none of which unmarshal).
166168
func ociAnnotationID(manifest *oci.ManifestContainer) string {
167-
if mj, ok := manifest.Manifest.Annotations["dev.containers.metadata"]; ok && mj != "" {
168-
var meta features.Feature
169-
if json.Unmarshal([]byte(mj), &meta) == nil {
170-
return meta.ID
171-
}
169+
var meta features.Feature
170+
if json.Unmarshal([]byte(manifest.Manifest.Annotations["dev.containers.metadata"]), &meta) != nil {
171+
return ""
172172
}
173-
return ""
173+
return meta.ID
174174
}
175175

176176
func readLocalFeatureMetadata(dir string) (features.Feature, error) {

‎internal/cli/feature_install.go‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -912,11 +912,12 @@ func extractTarGz(archivePath, destDir string) error {
912912

913913
// Tar paths always use forward slashes. Localize rejects absolute paths,
914914
// parent traversal, and names that cannot be represented safely on the
915-
// current platform. Root also prevents escapes through symlinks already
916-
// present below destDir.
915+
// current platform; what it returns is always local, as filepath.IsLocal
916+
// reports. Root also prevents escapes through symlinks already present
917+
// below destDir.
917918
cleanName := path.Clean(header.Name)
918919
localName, err := filepath.Localize(cleanName)
919-
if err != nil || !filepath.IsLocal(localName) {
920+
if err != nil {
920921
return fmt.Errorf("tar entry %q escapes the destination directory", header.Name)
921922
}
922923

@@ -992,9 +993,6 @@ func (e *digestMismatchError) Error() string {
992993
// It returns the tarball bytes and their computed "sha256:<hex>" digest so the
993994
// caller can pin/verify integrity (TS records and checks this digest).
994995
func downloadFeatureTarball(ctx context.Context, logger log.Logger, tarballURL string, env map[string]string) ([]byte, string, error) {
995-
if ctx == nil {
996-
ctx = context.Background()
997-
}
998996
req, err := http.NewRequestWithContext(ctx, http.MethodGet, tarballURL, nil)
999997
if err != nil {
1000998
return nil, "", err

0 commit comments

Comments
 (0)