Skip to content

Commit

Permalink
Do not register local and link components in pnpm9 detector
Browse files Browse the repository at this point in the history
  • Loading branch information
FernandoRojo committed Dec 19, 2024
1 parent 8a744bf commit 17790c1
Show file tree
Hide file tree
Showing 6 changed files with 117 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,19 @@ public bool IsLocalDependency(KeyValuePair<string, string> dependency)
}

/// <summary>
/// Parse a pnpm path of the form "/package-name/version".
/// Parse a pnpm path of the form "/package-name/version and create an npm component".
/// </summary>
/// <param name="pnpmPackagePath">a pnpm path of the form "/package-name/version".</param>
/// <returns>Data parsed from path.</returns>
public abstract DetectedComponent CreateDetectedComponentFromPnpmPath(string pnpmPackagePath);

/// <summary>
/// Parse a pnpm path of the form "/package-name/version into a packageName and Version.
/// </summary>
/// <param name="pnpmPackagePath">a pnpm path of the form "/package-name/version".</param>
/// <returns>Data parsed from path.</returns>
public abstract (string FullPackageName, string PackageVersion) ExtractNameAndVersionFromPnpmPackagePath(string pnpmPackagePath);

public virtual string ReconstructPnpmDependencyPath(string dependencyName, string dependencyVersion)
{
if (dependencyVersion.StartsWith('/'))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public override DetectedComponent CreateDetectedComponentFromPnpmPath(string pnp
return new DetectedComponent(new NpmComponent(parentName, parentVersion));
}

private (string Name, string Version) ExtractNameAndVersionFromPnpmPackagePath(string pnpmPackagePath)
public override (string FullPackageName, string PackageVersion) ExtractNameAndVersionFromPnpmPackagePath(string pnpmPackagePath)
{
var pnpmComponentDefSections = pnpmPackagePath.Trim('/').Split('/');
(var packageVersion, var indexVersionIsAt) = this.GetPackageVersion(pnpmComponentDefSections);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ public override DetectedComponent CreateDetectedComponentFromPnpmPath(string pnp

// Strip parenthesized suffices from package. These hold peed dep related information that is unneeded here.
// An example of a dependency path with these: /webpack-cli@4.10.0(webpack-bundle-analyzer@4.10.1)(webpack-dev-server@4.6.0)(webpack@5.89.0)
var (normalizedPackageName, packageVersion) = this.ExtractNameAndVersionFromPnpmPackagePath(pnpmPackagePath);
return new DetectedComponent(new NpmComponent(normalizedPackageName, packageVersion));
}

public override (string FullPackageName, string PackageVersion) ExtractNameAndVersionFromPnpmPackagePath(string pnpmPackagePath)
{
var fullPackageNameAndVersion = pnpmPackagePath.Split("(")[0];

var packageNameParts = fullPackageNameAndVersion.Split("@");
Expand All @@ -37,6 +43,6 @@ public override DetectedComponent CreateDetectedComponentFromPnpmPath(string pnp
// It is unclear if real packages could have a name starting with `/`, so avoid `TrimStart` that just in case.
var normalizedPackageName = fullPackageName[1..];

return new DetectedComponent(new NpmComponent(normalizedPackageName, packageVersion));
return (normalizedPackageName, packageVersion);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,23 @@ public class PnpmV9ParsingUtilities<T> : PnpmParsingUtilitiesBase<T>
where T : PnpmYaml
{
public override DetectedComponent CreateDetectedComponentFromPnpmPath(string pnpmPackagePath)
{
/*
* The format is documented at https://github.com/pnpm/spec/blob/master/dependency-path.md.
* At the writing it does not seem to reflect changes which were made in lock file format v9:
* See https://github.com/pnpm/spec/issues/5.
* In general, the spec sheet for the v9 lockfile is not published, so parsing of this lockfile was emperically determined.
* see https://github.com/pnpm/spec/issues/6
*/

// Strip parenthesized suffices from package. These hold peed dep related information that is unneeded here.
// An example of a dependency path with these: /webpack-cli@4.10.0(webpack-bundle-analyzer@4.10.1)(webpack-dev-server@4.6.0)(webpack@5.89.0)
(var fullPackageName, var packageVersion) = this.ExtractNameAndVersionFromPnpmPackagePath(pnpmPackagePath);

return new DetectedComponent(new NpmComponent(fullPackageName, packageVersion));
}

public override (string FullPackageName, string PackageVersion) ExtractNameAndVersionFromPnpmPackagePath(string pnpmPackagePath)
{
/*
* The format is documented at https://github.com/pnpm/spec/blob/master/dependency-path.md.
Expand All @@ -28,7 +45,7 @@ public override DetectedComponent CreateDetectedComponentFromPnpmPath(string pnp
// Version is section after last `@`.
var packageVersion = packageNameParts[^1];

return new DetectedComponent(new NpmComponent(fullPackageName, packageVersion));
return (fullPackageName, packageVersion);
}

/// <summary>
Expand Down
27 changes: 14 additions & 13 deletions src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,8 @@ public void RecordDependencyGraphFromFile(string yamlFileContent, ISingleFileCom
// Ignore "file:" as these are local packages.
// Such local packages should only be referenced at the top level (via ProcessDependencyList) which also skips them or from other local packages (which this skips).
// There should be no cases where a non-local package references a local package, so skipping them here should not result in failed lookups below when adding all the graph references.
if (pnpmDependencyPath.StartsWith(PnpmConstants.PnpmFileDependencyPath))
{
continue;
}
var (packageName, packageVersion) = this.pnpmParsingUtilities.ExtractNameAndVersionFromPnpmPackagePath(pnpmDependencyPath);
var isFileOrLink = packageVersion.StartsWith(PnpmConstants.PnpmLinkDependencyPath) || packageVersion.StartsWith(PnpmConstants.PnpmFileDependencyPath);

var dependencyPath = pnpmDependencyPath;
if (pnpmDependencyPath.StartsWith('/'))
Expand All @@ -48,7 +46,10 @@ public void RecordDependencyGraphFromFile(string yamlFileContent, ISingleFileCom
// It should get registered again with with additional information (what depended on it) later,
// but registering it now ensures nothing is missed due to a limitation in dependency traversal
// like skipping local dependencies which might have transitively depended on this.
singleFileComponentRecorder.RegisterUsage(parentDetectedComponent);
if (!isFileOrLink)
{
singleFileComponentRecorder.RegisterUsage(parentDetectedComponent);
}
}

// now that the components dictionary is populated, add direct dependencies of the current file/project setting isExplicitReferencedDependency to true
Expand All @@ -70,20 +71,20 @@ private void ProcessDependencyList(ISingleFileComponentRecorder singleFileCompon
{
foreach (var (name, dep) in dependencies ?? Enumerable.Empty<KeyValuePair<string, PnpmYamlV9Dependency>>())
{
// Ignore "file:" and "link:" as these are local packages.
if (dep.Version.StartsWith(PnpmConstants.PnpmLinkDependencyPath) || dep.Version.StartsWith(PnpmConstants.PnpmFileDependencyPath))
{
continue;
}

var pnpmDependencyPath = this.pnpmParsingUtilities.ReconstructPnpmDependencyPath(name, dep.Version);
var (component, package) = components[pnpmDependencyPath];

// Lockfile v9 apparently removed the tagging of dev dependencies in the lockfile, so we revert to using the dependency tree to establish dev dependency state.
// At this point, the root dependencies are marked according to which dependency group they are declared in the lockfile itself.
singleFileComponentRecorder.RegisterUsage(component, isExplicitReferencedDependency: true, isDevelopmentDependency: isDevelopmentDependency);
// Ignore "file:" and "link:" as these are local packages.
var isFileOrLink = dep.Version.StartsWith(PnpmConstants.PnpmLinkDependencyPath) || dep.Version.StartsWith(PnpmConstants.PnpmFileDependencyPath);
if (!isFileOrLink)
{
singleFileComponentRecorder.RegisterUsage(component, isExplicitReferencedDependency: true, isDevelopmentDependency: isDevelopmentDependency);
}

var seenDependencies = new HashSet<string>();
this.ProcessIndirectDependencies(singleFileComponentRecorder, components, component.Component.Id, package.Dependencies, isDevelopmentDependency, seenDependencies);
this.ProcessIndirectDependencies(singleFileComponentRecorder, components, isFileOrLink ? null : component.Component.Id, package.Dependencies, isDevelopmentDependency, seenDependencies);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,75 @@ public async Task TestPnpmDetector_V9_GoodLockVersion_SkipsFileAndLinkDependenci
parentComponent => parentComponent.Name == "sampleDependency");
}

[TestMethod]
public async Task TestPnpmDetector_V9_GoodLockVersion_FileAndLinkDependenciesAreNotRegistered()
{
var yamlFile = @"
lockfileVersion: '9.0'
settings:
autoInstallPeers: true
excludeLinksFromLockfile: false
importers:
.:
dependencies:
sampleDependency:
specifier: ^1.1.1
version: 1.1.1
sampleFileDependency:
specifier: file:../test
version: file:../test
SampleLinkDependency:
specifier: workspace:*
version: link:SampleLinkDependency
packages:
sampleDependency@1.1.1:
resolution: {integrity: sha512-zRpUiDwd/xk6ADqPMATG8vc9VPrkck7T07OIx0gnjmJAnHnTVXNQG3vfvWNuiZIkwu9KrKdA1iJKfsfTVxE6NA==}
sampleIndirectDependency2@2.2.2:
resolution: {integrity: sha512-FQN4MRfuJeHf7cBbBMJFXhKSDq+2kAArBlmRBvcvFE5BB1HZKXtSFASDhdlz9zOYwxh8lDdnvmMOe/+5cdoEdg==}
engines: {node: '>= 0.8'}
sampleIndirectDependency@3.3.3:
resolution: {integrity: sha512-ZySD7Nf91aLB0RxL4KGrKHBXl7Eds1DAmEdcoVawXnLD7SDhpNgtuII2aAkg7a7QS41jxPSZ17p4VdGnMHk3MQ==}
engines: {node: '>=0.4.0'}
snapshots:
sampleDependency@1.1.1:
dependencies:
sampleIndirectDependency: 3.3.3
sampleIndirectDependency2: 2.2.2
'file://../sampleFile': 'link:../\\'
sampleIndirectDependency2@2.2.2: {}
sampleIndirectDependency@3.3.3: {}
sampleFileDependency@file:../test': {}
";

var (scanResult, componentRecorder) = await this.DetectorTestUtility
.WithFile("pnpm-lock.yaml", yamlFile)
.ExecuteDetectorAsync();

scanResult.ResultCode.Should().Be(ProcessingResultCode.Success);

var detectedComponents = componentRecorder.GetDetectedComponents();
detectedComponents.Should().HaveCount(3);
var npmComponents = detectedComponents.Select(x => new { Component = x.Component as NpmComponent, DetectedComponent = x });
npmComponents.Should().Contain(x => x.Component.Name == "sampleDependency" && x.Component.Version == "1.1.1");
npmComponents.Should().Contain(x => x.Component.Name == "sampleIndirectDependency2" && x.Component.Version == "2.2.2");
npmComponents.Should().Contain(x => x.Component.Name == "sampleIndirectDependency" && x.Component.Version == "3.3.3");

var noDevDependencyComponent = npmComponents.First(x => x.Component.Name == "sampleDependency");
var indirectDependencyComponent2 = npmComponents.First(x => x.Component.Name == "sampleIndirectDependency2");
var indirectDependencyComponent = npmComponents.First(x => x.Component.Name == "sampleIndirectDependency");

componentRecorder.GetEffectiveDevDependencyValue(noDevDependencyComponent.Component.Id).Should().BeFalse();
componentRecorder.GetEffectiveDevDependencyValue(indirectDependencyComponent2.Component.Id).Should().BeFalse();
componentRecorder.GetEffectiveDevDependencyValue(indirectDependencyComponent.Component.Id).Should().BeFalse();
componentRecorder.AssertAllExplicitlyReferencedComponents<NpmComponent>(
indirectDependencyComponent.Component.Id,
parentComponent => parentComponent.Name == "sampleDependency");
componentRecorder.AssertAllExplicitlyReferencedComponents<NpmComponent>(
indirectDependencyComponent2.Component.Id,
parentComponent => parentComponent.Name == "sampleDependency");
}

[TestMethod]
public async Task TestPnpmDetector_V9_GoodLockVersion_MissingSnapshotsSuccess()
{
Expand Down

0 comments on commit 17790c1

Please sign in to comment.