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

Improve DetectUnpinnedDotnetToolInstallVersions function #110

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
12 changes: 7 additions & 5 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ jobs:
sudo apt install --yes --no-install-recommends npm curl
# need to update nodejs because with ubuntu's default nodejs version we would get this error:
# error @jest/core@29.4.1: The engine "node" is incompatible with this module. Expected version "^14.15.0 || ^16.10.0 || >=18.0.0". Got "12.22.9"
sudo npm install --global n
sudo npm install --global n@9.1.0
Copy link
Member

Choose a reason for hiding this comment

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

@realmarv can we use stable instead?

Copy link
Contributor Author

@tehraninasab tehraninasab Jul 6, 2023

Choose a reason for hiding this comment

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

you mean @latest?

Copy link
Member

Choose a reason for hiding this comment

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

no, in fact if using @latest, this script should fail as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there is no @stable tag

sudo n lts
- name: Print versions
run: |
Expand All @@ -86,12 +86,12 @@ jobs:
npm --version
- name: Install yarn
run: |
npm install --global yarn
npm install --global yarn@1.22.19
yarn add --dev jest typescript ts-jest @types/jest
- name: Install commitlint
run: |
npm install conventional-changelog-conventionalcommits
npm install commitlint@latest
npm install conventional-changelog-conventionalcommits@6.1.0
npm install commitlint@17.6.6
- name: Print versions
run: |
git --version
Expand Down Expand Up @@ -130,7 +130,7 @@ jobs:
# at wrapSafe (internal/modules/cjs/loader.js:915:16)
# ...
# ```
sudo npm install --global n
sudo npm install --global n@9.1.0
sudo n lts
- uses: actions/checkout@v2
with:
Expand Down Expand Up @@ -179,6 +179,8 @@ jobs:
run: dotnet fsi scripts/unpinnedNugetPackageReferenceVersions.fsx
- name: Check there are no unpinned versions in `dotnet tool install` commands
run: dotnet fsi scripts/unpinnedDotnetToolInstallVersions.fsx
- name: Check there are no unpinned versions in `npm install` commands
run: dotnet fsi scripts/unpinnedNpmPackageInstallVersions.fsx
- name: Check commits 1 by 1
if: github.event_name == 'pull_request'
run: dotnet fsi scripts/checkCommits1by1.fsx
Expand Down
20 changes: 20 additions & 0 deletions scripts/unpinnedNpmPackageInstallVersions.fsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/usr/bin/env -S dotnet fsi

open System
open System.IO

#load "../src/FileConventions/Library.fs"
#load "../src/FileConventions/Helpers.fs"

let rootDir = Path.Combine(__SOURCE_DIRECTORY__, "..") |> DirectoryInfo

let invalidFiles =
Helpers.GetInvalidFiles
rootDir
"*.yml"
FileConventions.DetectUnpinnedNpmPackageInstallVersions

let message =
"Please define the package version number in the `npm install` commands."

Helpers.AssertNoInvalidFiles invalidFiles message
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
name: CI

on: [push, pull_request]

jobs:
file-conventions:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Install prettier without specifying its version
run: npm install prettier
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
name: CI

on: [push, pull_request]

jobs:
file-conventions:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Install an npm package without specifying its version
run: sudo npm install --save-dev @prettier/plugin-xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
name: CI

on: [push, pull_request]

jobs:
file-conventions:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Install two npm packages, one with version and the other one without version
run: sudo npm install --save-dev @prettier/plugin-xml prettier@2.4.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
name: CI

on: [push, pull_request]

jobs:
build:
name: Build
runs-on: ubuntu-22.04
container:
image: "ubuntu:22.04"
steps:
- name: Install fantomless-tool
run: |
dotnet tool install fantomless-tool --version 4.8.999
- name: Print "Hello World!"
run: echo "Hello World"
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
name: CI

on: [push, pull_request]

jobs:
file-conventions:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Install prettier with specifying its version
run: npm install prettier@2.8.3
79 changes: 79 additions & 0 deletions src/FileConventions.Test/FileConventions.Test.fs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,85 @@ let DetectUnpinnedDotnetToolInstallVersions1() =
Is.EqualTo true
)

[<Test>]
let DetectUnpinnedDotnetToolInstallVersions2() =
let fileInfo =
(FileInfo(
Path.Combine(
dummyFilesDirectory.FullName,
"DummyCIWithoutUnpinnedDotnetToolInstallVersion.yml"
)
))

Assert.That(
DetectUnpinnedDotnetToolInstallVersions fileInfo,
Is.EqualTo false
)


[<Test>]
let DetectUnpinnedNpmPackageInstallVersions1() =
let fileInfo =
(FileInfo(
Path.Combine(
dummyFilesDirectory.FullName,
"DummyCIWithUnpinnedNpmPackageInstallVersion1.yml"
)
))

Assert.That(
DetectUnpinnedNpmPackageInstallVersions fileInfo,
Is.EqualTo true
)


[<Test>]
let DetectUnpinnedNpmPackageInstallVersions2() =
let fileInfo =
(FileInfo(
Path.Combine(
dummyFilesDirectory.FullName,
"DummyCIWithoutUnpinnedNpmPackageInstallVersion.yml"
)
))

Assert.That(
DetectUnpinnedNpmPackageInstallVersions fileInfo,
Is.EqualTo false
)


[<Test>]
let DetectUnpinnedNpmPackageInstallVersions3() =
let fileInfo =
(FileInfo(
Path.Combine(
dummyFilesDirectory.FullName,
"DummyCIWithUnpinnedNpmPackageInstallVersion2.yml"
)
))

Assert.That(
DetectUnpinnedNpmPackageInstallVersions fileInfo,
Is.EqualTo true
)


[<Test>]
let DetectUnpinnedNpmPackageInstallVersions4() =
let fileInfo =
(FileInfo(
Path.Combine(
dummyFilesDirectory.FullName,
"DummyCIWithUnpinnedNpmPackageInstallVersion3.yml"
)
))

Assert.That(
DetectUnpinnedNpmPackageInstallVersions fileInfo,
Is.EqualTo true
)


[<Test>]
let DetectAsteriskInPackageReferenceItems1() =
Expand Down
38 changes: 36 additions & 2 deletions src/FileConventions/Library.fs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ let DetectUnpinnedVersionsInGitHubCI(fileInfo: FileInfo) =
latestTagInRunsOnRegex.IsMatch fileText

let DetectUnpinnedDotnetToolInstallVersions(fileInfo: FileInfo) =
assert (fileInfo.FullName.EndsWith(".yml"))
assert fileInfo.FullName.EndsWith ".yml"

let fileLines = File.ReadLines fileInfo.FullName

Expand All @@ -60,12 +60,46 @@ let DetectUnpinnedDotnetToolInstallVersions(fileInfo: FileInfo) =
fileLines
|> Seq.filter(fun line -> dotnetToolInstallRegex.IsMatch line)
|> Seq.filter(fun line ->
not(line.Contains("--version")) && not(line.Contains("-v"))
not(line.Contains "--version") && not(line.Contains "-v")
)
|> (fun unpinnedVersions -> Seq.length unpinnedVersions > 0)

unpinnedDotnetToolInstallVersions

let DetectUnpinnedNpmPackageInstallVersions(fileInfo: FileInfo) =
assert fileInfo.FullName.EndsWith ".yml"

let fileLines = File.ReadLines fileInfo.FullName

let npmPackageInstallRegex =
Regex("npm\\s+install\\s+", RegexOptions.Compiled)

let npmPackageVersionRegex =
Regex("@\\d+\\.\\d+\\.\\d+", RegexOptions.Compiled)

let unpinnedNpmPackageInstallVersions =
fileLines
|> Seq.filter(fun line -> npmPackageInstallRegex.IsMatch line)
|> Seq.filter(fun line ->
let npmPackagesRegex =
Regex("(?<=npm install ).*$", RegexOptions.Compiled)

let npmInstallPackages = npmPackagesRegex.Match line

let numNpmInstallPackages =
npmInstallPackages.Value.Split(" ")
|> Seq.filter(fun word -> word.Trim().StartsWith("-") |> not)
|> Seq.length

let numNpmInstallVersions =
npmPackageVersionRegex.Matches line |> Seq.length

numNpmInstallPackages = numNpmInstallVersions |> not
)
|> (fun unpinnedVersions -> Seq.length unpinnedVersions > 0)

unpinnedNpmPackageInstallVersions

let DetectAsteriskInPackageReferenceItems(fileInfo: FileInfo) =
assert (fileInfo.FullName.EndsWith "proj")

Expand Down
Loading