-
Notifications
You must be signed in to change notification settings - Fork 10
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
Easy dotnet conventions to verify #139
Changes from 6 commits
50fe215
186213e
2ebd2b5
cc82850
a0272d6
482d4d6
5f06394
12f4d0b
b0acb0c
7823ab2
9b8afb3
21f3749
81bfb57
e124a91
eb5f76c
885a60a
4e0c6e1
6e80056
46f3dae
45e298b
c190f9c
d49a99c
19b442d
e6fc879
5358b96
ce4c0c6
18b828e
1fd7576
a326d09
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
#!/usr/bin/env -S dotnet fsi | ||
|
||
open System | ||
open System.IO | ||
|
||
let emptyString = String.Empty |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
#!/usr/bin/env -S dotnet fsi | ||
|
||
open System | ||
open System.IO | ||
|
||
let emptyString = "" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
namespace Foo |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
namespace Baz |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -586,3 +586,87 @@ let IsExecutableTest2() = | |
)) | ||
|
||
Assert.That(IsExecutable fileInfo, Is.EqualTo false) | ||
|
||
|
||
[<Test>] | ||
let DefiningEmptyStringsWithDoubleQuotes1() = | ||
let fileInfo = | ||
(FileInfo( | ||
Path.Combine( | ||
dummyFilesDirectory.FullName, | ||
"DummyScriptWithConventionalEmptyString.fsx" | ||
) | ||
)) | ||
|
||
Assert.That(DefiningEmptyStringsWithDoubleQuotes fileInfo, Is.EqualTo false) | ||
|
||
|
||
[<Test>] | ||
let DefiningEmptyStringsWithDoubleQuotes2() = | ||
let fileInfo = | ||
(FileInfo( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. extra parens |
||
Path.Combine( | ||
dummyFilesDirectory.FullName, | ||
"DummyScriptWithNonConventionalEmptyString.fsx" | ||
) | ||
)) | ||
|
||
Assert.That(DefiningEmptyStringsWithDoubleQuotes fileInfo, Is.EqualTo true) | ||
|
||
|
||
[<Test>] | ||
let ProjFilesNamingConvention1() = | ||
let fileInfo = | ||
(FileInfo( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extra paren |
||
Path.Combine( | ||
dummyFilesDirectory.FullName, | ||
"DummyProjFileWithTheSameNameAsItsParentFolder", | ||
"DummyProjFileWithTheSameNameAsItsParentFolder.fsproj" | ||
) | ||
)) | ||
|
||
Assert.That(ProjFilesNamingConvention fileInfo, Is.EqualTo false) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assert.False? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nope |
||
|
||
|
||
[<Test>] | ||
let ProjFilesNamingConvention2() = | ||
let fileInfo = | ||
(FileInfo( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extra paren. |
||
Path.Combine( | ||
dummyFilesDirectory.FullName, | ||
"DummyProject", | ||
"DummyProjFileWithoutTheSameNameAsItsParentFolder.fsproj" | ||
) | ||
)) | ||
|
||
Assert.That(ProjFilesNamingConvention fileInfo, Is.EqualTo true) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't it easier to use Assert.True? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, Assert.That is fine |
||
|
||
|
||
[<Test>] | ||
let NamespaceConvention1() = | ||
let fileInfo = | ||
(FileInfo( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. extra paren |
||
Path.Combine( | ||
dummyFilesDirectory.FullName, | ||
"src", | ||
"Foo", | ||
"DummyFileUnderFooWithRightNamespace.fs" | ||
) | ||
)) | ||
|
||
Assert.That(NotFollowingNamespaceConvention fileInfo, Is.EqualTo false) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assert.False? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nope |
||
|
||
|
||
[<Test>] | ||
let NamespaceConvention2() = | ||
let fileInfo = | ||
(FileInfo( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. extra paren |
||
Path.Combine( | ||
dummyFilesDirectory.FullName, | ||
"src", | ||
"Foo", | ||
"DummyFileUnderFooWithWrongNamespace.fs" | ||
) | ||
)) | ||
|
||
Assert.That(NotFollowingNamespaceConvention fileInfo, Is.EqualTo true) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assert.True? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nope |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -390,3 +390,52 @@ let NonVerboseFlags(fileInfo: FileInfo) = | |
let IsExecutable(fileInfo: FileInfo) = | ||
let hasExecuteAccess = Syscall.access(fileInfo.FullName, AccessModes.X_OK) | ||
hasExecuteAccess = 0 | ||
|
||
let DefiningEmptyStringsWithDoubleQuotes(fileInfo: FileInfo) = | ||
let fileText = File.ReadAllText fileInfo.FullName | ||
fileText.Contains "\"\"" | ||
|
||
let ProjFilesNamingConvention(fileInfo: FileInfo) = | ||
let regex = new Regex("(.*)\..*proj$") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need for new keyword There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also extra paren. |
||
assert (regex.IsMatch fileInfo.FullName) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure about this but I think paren is not needed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it is needed; because There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. assert is for debug mode only, are we sure it should be used here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Not sure because it compiles without them 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aarani you're right, they should use Fsdk's Misc.BetterAssert |
||
let fileName = Path.GetFileNameWithoutExtension fileInfo.FullName | ||
|
||
let parentDirectoryName = | ||
Path.GetDirectoryName fileInfo.FullName |> Path.GetFileName | ||
|
||
printfn | ||
"File name: %s, Parent directory name: %s" | ||
fileName | ||
parentDirectoryName | ||
|
||
fileName <> parentDirectoryName | ||
|
||
let NotFollowingNamespaceConvention(fileInfo: FileInfo) = | ||
assert (fileInfo.FullName.EndsWith(".fs")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. extra paren |
||
|
||
let fileName = Path.GetFileNameWithoutExtension fileInfo.FullName | ||
|
||
let parentDirectoryName = | ||
Path.GetDirectoryName fileInfo.FullName |> Path.GetFileName | ||
|
||
printfn | ||
"File name: %s, Parent directory name: %s" | ||
fileName | ||
parentDirectoryName | ||
|
||
if parentDirectoryName <> "src" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know the logic is based on the corresponding issue, but for instance in NX we have multiple projects in "tests" folder that this rule can be applied to them. Can't we modify the rule to search for the folders containing projects? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. later |
||
&& fileInfo.FullName.Contains | ||
$"{Path.DirectorySeparatorChar}src{Path.DirectorySeparatorChar}" then | ||
let fileText = File.ReadLines fileInfo.FullName | ||
|
||
if fileText.Any() then | ||
let firstLine = fileText.First() | ||
|
||
if firstLine.Contains "namespace" then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not something like this?
|
||
firstLine.Contains parentDirectoryName |> not | ||
else | ||
false | ||
else | ||
false | ||
else | ||
false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra parens