Skip to content

Commit

Permalink
Filter out non-design time capable projects (#225)
Browse files Browse the repository at this point in the history
* fantomas livves

* add failing tests

* Build design time projects only
  • Loading branch information
TheAngryByrd authored Feb 15, 2025
1 parent a6d01ee commit 65281d7
Show file tree
Hide file tree
Showing 10 changed files with 189 additions and 52 deletions.
14 changes: 11 additions & 3 deletions .config/dotnet-tools.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
{
"version": 1,
"isRoot": true,
"tools": {}
"version": 1,
"isRoot": true,
"tools": {
"fantomas": {
"version": "7.0.0",
"commands": [
"fantomas"
],
"rollForward": false
}
}
}
118 changes: 69 additions & 49 deletions src/Ionide.ProjInfo/Library.fs
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,14 @@ module SdkDiscovery =
|> Seq.toArray

p.WaitForExit()

if p.ExitCode = 0 then
output
elif failOnError then
let output = output |> String.concat "\n"
let output =
output
|> String.concat "\n"

failwith $"`{binaryFullPath.FullName} {args}` failed with exit code %i{p.ExitCode}. Output: %s{output}"
else
// for the legacy VS flow, whose behaviour is harder to test, we maintain compatibility with how proj-info
Expand Down Expand Up @@ -340,6 +344,20 @@ module ProjectLoader =
internal
| StandardProject of ProjectInstance
| TraversalProject of ProjectInstance
/// This could be things like shproj files, or other things that aren't standard projects
| Other of ProjectInstance

let (|IsTraversal|_|) (p: ProjectInstance) =
match p.GetProperty "IsTraversal" with
| null -> None
| p when Boolean.TryParse(p.EvaluatedValue) = (true, false) -> None
| _ -> Some()

let (|DesignTimeCapable|_|) (targets: string seq) (p: ProjectInstance) =
if Seq.forall p.Targets.ContainsKey targets then
Some()
else
None

let internal projectLoaderLogger = lazy (LogProvider.getLoggerByName "ProjectLoader")

Expand Down Expand Up @@ -411,11 +429,12 @@ module ProjectLoader =
match _message with
| null ->
// if the project is already loaded throw a nicer message
let message = System.Text.StringBuilder()
let message = Text.StringBuilder()
let appendLine (t: string) (sb: Text.StringBuilder) = sb.AppendLine t

message
.AppendLine($"The project '{projectPath}' already exists in the project collection with the same global properties.")
.AppendLine("The global properties requested were:")
|> appendLine $"The project '{projectPath}' already exists in the project collection with the same global properties."
|> appendLine "The global properties requested were:"
|> ignore

for (KeyValue(k, v)) in properties do
Expand Down Expand Up @@ -617,7 +636,7 @@ module ProjectLoader =
let lines: seq<string> = File.ReadLines path

(Seq.tryFind (fun (line: string) -> line.Contains legacyProjFormatXmlns) lines)
.IsSome
|> Option.isSome
else
false

Expand All @@ -639,22 +658,20 @@ module ProjectLoader =
let loggers = createLoggers [ path ] binaryLogs sw

let pi = project.CreateProjectInstance()
let designTimeTargets = designTimeBuildTargets isLegacyFrameworkProjFile

let doDesignTimeBuild () =
let build = pi.Build(designTimeBuildTargets isLegacyFrameworkProjFile, loggers)
let build = pi.Build(designTimeTargets, loggers)

if build then
Ok(StandardProject pi)
else
Error(sw.ToString())

let yieldTraversalProject () = Ok(TraversalProject pi)

// do traversal project detection here
match pi.GetProperty "IsTraversal" with
| null -> doDesignTimeBuild ()
| p when Boolean.Parse(p.EvaluatedValue) = false -> doDesignTimeBuild ()
| _ -> yieldTraversalProject ()
match pi with
| IsTraversal -> Ok(TraversalProject pi)
| DesignTimeCapable designTimeTargets -> doDesignTimeBuild ()
| _ -> Ok(Other pi)

with exc ->
projectLoaderLogger.Value.error (
Expand Down Expand Up @@ -723,6 +740,7 @@ module ProjectLoader =
}
)
|> Seq.toList
| Other(_) -> List.empty

let getCompileItems (p: ProjectInstance) =
p.Items
Expand Down Expand Up @@ -945,6 +963,7 @@ module ProjectLoader =
type LoadedProjectInfo =
| StandardProjectInfo of ProjectOptions
| TraversalProjectInfo of ProjectReference list
| OtherProjectInfo of ProjectInstance

let getLoadedProjectInfo (path: string) customProperties project : Result<LoadedProjectInfo, string> =
// let (LoadedProject p) = project
Expand Down Expand Up @@ -1001,6 +1020,7 @@ module ProjectLoader =
else
let proj = mapToProject path commandLineArgs p2pRefs compileItems nuGetRefs sdkInfo props customProps
Ok(LoadedProjectInfo.StandardProjectInfo proj)
| LoadedProject.Other p -> Ok(LoadedProjectInfo.OtherProjectInfo p)

/// A type that turns project files or solution files into deconstructed options.
/// Use this in conjunction with the other ProjInfo libraries to turn these options into
Expand Down Expand Up @@ -1105,42 +1125,32 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri
paths
|> Seq.iter (fun p -> loadingNotification.Trigger(WorkspaceProjectState.Loading p))

let designTimeTargets = ProjectLoader.designTimeBuildTargets false

let graph =
match
let entryPoints =
paths
|> Seq.map ProjectGraphEntryPoint
|> List.ofSeq
with
| [ x ] ->
let g: ProjectGraph =
ProjectGraph(x, projectCollection = per_request_collection, projectInstanceFactory = projectInstanceFactory)
// When giving ProjectGraph a singular project, g.EntryPointNodes only contains that project.
// To get it to build the Graph with all the dependencies we need to look at all the ProjectNodes
// and tell the graph to use all as potentially an entrypoint
let nodes =
g.ProjectNodes
|> Seq.choose (fun pn ->
match pn.ProjectInstance.GetProperty("IsTraversal") with
| null ->
ProjectGraphEntryPoint pn.ProjectInstance.FullPath
|> Some
| p ->
match bool.TryParse(p.EvaluatedValue) with
| true, true -> None
| true, false ->
ProjectGraphEntryPoint pn.ProjectInstance.FullPath
|> Some
| false, _ -> None
)

ProjectGraph(nodes, projectCollection = per_request_collection, projectInstanceFactory = projectInstanceFactory)

| xs ->
let entryPoints =
paths
|> Seq.map ProjectGraphEntryPoint
|> List.ofSeq

let g: ProjectGraph =
ProjectGraph(entryPoints, projectCollection = per_request_collection, projectInstanceFactory = projectInstanceFactory)
// When giving ProjectGraph a singular project, g.EntryPointNodes only contains that project.
// To get it to build the Graph with all the dependencies we need to look at all the ProjectNodes
// and tell the graph to use all as potentially an entrypoint
// Additionally, we need to filter out any projects that are not design time capable
let nodes =
g.ProjectNodes
|> Seq.choose (fun pn ->
match pn.ProjectInstance with
| ProjectLoader.DesignTimeCapable designTimeTargets ->

ProjectGraphEntryPoint pn.ProjectInstance.FullPath
|> Some
| _ -> None
)

ProjectGraph(nodes, projectCollection = per_request_collection, projectInstanceFactory = projectInstanceFactory)

graph

Expand Down Expand Up @@ -1205,9 +1215,11 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri
let gbr =
GraphBuildRequestData(
projectGraph = projects,
targetsToBuild=ProjectLoader.designTimeBuildTargets false,
hostServices=null,
flags= (BuildRequestDataFlags.ReplaceExistingProjectInstance ||| BuildRequestDataFlags.ClearCachesAfterBuild)
targetsToBuild = ProjectLoader.designTimeBuildTargets false,
hostServices = null,
flags =
(BuildRequestDataFlags.ReplaceExistingProjectInstance
||| BuildRequestDataFlags.ClearCachesAfterBuild)
)

let bm = BuildManager.DefaultBuildManager
Expand Down Expand Up @@ -1260,7 +1272,6 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri
let projects =
builtProjects
|> Seq.map (fun p -> p.FullPath, ProjectLoader.getLoadedProjectInfo p.FullPath customProperties (ProjectLoader.StandardProject p))

|> Seq.choose (fun (projectPath, projectOptionResult) ->
match projectOptionResult with
| Ok projectOptions ->
Expand Down Expand Up @@ -1303,6 +1314,12 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri
)

loadingNotification.Trigger(WorkspaceProjectState.Loaded(po, allStandardProjects, false))
| ProjectLoader.LoadedProjectInfo.OtherProjectInfo p ->
logger.info (
Log.setMessage "Other project loaded {project}"
>> Log.addContextDestructured "project" p.FullPath
)

)

allStandardProjects :> seq<_>
Expand Down Expand Up @@ -1417,6 +1434,7 @@ type WorkspaceLoader private (toolsPath: ToolsPath, ?globalProperties: (string *
match project with
| ProjectLoader.LoadedProjectInfo.StandardProjectInfo p -> p.ReferencedProjects
| ProjectLoader.LoadedProjectInfo.TraversalProjectInfo p -> p
| ProjectLoader.LoadedProjectInfo.OtherProjectInfo p -> []

let lst =
referencedProjects
Expand All @@ -1432,6 +1450,7 @@ type WorkspaceLoader private (toolsPath: ToolsPath, ?globalProperties: (string *
match project with
| ProjectLoader.LoadedProjectInfo.StandardProjectInfo p -> Some p
| ProjectLoader.LoadedProjectInfo.TraversalProjectInfo p -> None
| ProjectLoader.LoadedProjectInfo.OtherProjectInfo p -> None

lst, info
| Error msg ->
Expand All @@ -1446,13 +1465,14 @@ type WorkspaceLoader private (toolsPath: ToolsPath, ?globalProperties: (string *

match project with
| ProjectLoader.LoadedProjectInfo.StandardProjectInfo p -> loadingNotification.Trigger(WorkspaceProjectState.Loaded(p, getAllKnown (), true))

| ProjectLoader.LoadedProjectInfo.TraversalProjectInfo p -> ()
| ProjectLoader.LoadedProjectInfo.OtherProjectInfo p -> ()

let referencedProjects =
match project with
| ProjectLoader.LoadedProjectInfo.StandardProjectInfo p -> p.ReferencedProjects
| ProjectLoader.LoadedProjectInfo.TraversalProjectInfo p -> p
| ProjectLoader.LoadedProjectInfo.OtherProjectInfo p -> []

let lst =
referencedProjects
Expand Down
8 changes: 8 additions & 0 deletions test/Ionide.ProjInfo.Tests/TestAssets.fs
Original file line number Diff line number Diff line change
Expand Up @@ -321,3 +321,11 @@ let ``traversal project`` = {
yield! ``sample3 Netsdk projs``.ProjectReferences
]
}

let ``sample 11 sln with other project types`` = {
ProjDir = "sample11-solution-with-other-projects"
AssemblyName = ""
ProjectFile = "sample11-solution-with-other-projects.sln"
TargetFrameworks = Map.empty
ProjectReferences = []
}
31 changes: 31 additions & 0 deletions test/Ionide.ProjInfo.Tests/Tests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ let implAssemblyForProject (test: TestAssetProjInfo) = $"{test.AssemblyName}.dll
let refAssemblyForProject (test: TestAssetProjInfo) =
Path.Combine("ref", implAssemblyForProject test)

let getResult (r: Result<_, _>) =
match r with
| Ok x -> x
| Result.Error e -> failwithf "%A" e

let TestRunDir =
RepoDir
/ "test"
Expand Down Expand Up @@ -1999,6 +2004,7 @@ let debugTests toolsPath workspaceLoader (workspaceFactory: ToolsPath -> IWorksp
(fun logger fs ->

let loader = workspaceFactory toolsPath

let slnPath =
@"C:\Users\JimmyByrd\Documents\Repositories\public\TheAngryByrd\FsToolkit.ErrorHandling\FsToolkit.ErrorHandling.sln"

Expand Down Expand Up @@ -2245,6 +2251,28 @@ let traversalProjectTest toolsPath loaderType workspaceFactory =

)

let sample11OtherProjectsTest toolsPath loaderType workspaceFactory =
testCase
$"Can load sample11 with other projects like shproj in sln - {loaderType}"
(fun () ->

let projPath = pathForProject ``sample 11 sln with other project types``

let projPaths =
// using Inspectsln emulates what we do in FsAutocomplete for gathering projects to load
InspectSln.tryParseSln projPath
|> getResult
|> InspectSln.loadingBuildOrder

let loader: IWorkspaceLoader = workspaceFactory toolsPath

let parsed =
loader.LoadProjects projPaths
|> Seq.toList

Expect.hasLength parsed 1 "Should have fsproj"
)

let tests toolsPath =
let testSample3WorkspaceLoaderExpected = [
ExpectNotification.loading "c1.fsproj"
Expand Down Expand Up @@ -2371,4 +2399,7 @@ let tests toolsPath =

traversalProjectTest toolsPath (nameof (WorkspaceLoader)) WorkspaceLoader.Create
traversalProjectTest toolsPath (nameof (WorkspaceLoaderViaProjectGraph)) WorkspaceLoaderViaProjectGraph.Create

sample11OtherProjectsTest toolsPath (nameof (WorkspaceLoader)) WorkspaceLoader.Create
sample11OtherProjectsTest toolsPath (nameof (WorkspaceLoaderViaProjectGraph)) WorkspaceLoaderViaProjectGraph.Create
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
namespace classlibf1

module Say =
let hello name =
printfn "Hello %s" name
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>net8.0</TargetFramework>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
</PropertyGroup>

<ItemGroup>
<Compile Include="Library.fs" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@

Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio Version 17
VisualStudioVersion = 17.0.31903.59
MinimumVisualStudioVersion = 10.0.40219.1
Project("{F2A71F9B-5D33-465A-A702-920D77279786}") = "classlibf1", "classlibf1\classlibf1.fsproj", "{DEB2A50A-745D-4BB4-9DB5-D688F1FE8D7E}"
EndProject
Project("{D954291E-2A0B-460D-934E-DC6B0785DB48}") = "shared", "shared.shproj", "{AA1DD2E8-1CDE-4D1A-896C-8212C6D061A7}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Release|Any CPU = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
EndGlobalSection
GlobalSection(ProjectConfigurationPlatforms) = postSolution
{DEB2A50A-745D-4BB4-9DB5-D688F1FE8D7E}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{DEB2A50A-745D-4BB4-9DB5-D688F1FE8D7E}.Debug|Any CPU.Build.0 = Debug|Any CPU
{DEB2A50A-745D-4BB4-9DB5-D688F1FE8D7E}.Release|Any CPU.ActiveCfg = Release|Any CPU
{DEB2A50A-745D-4BB4-9DB5-D688F1FE8D7E}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
EndGlobal
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
open System
let lol = ()
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?xml version="1.0" encoding="utf-8"?>
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<PropertyGroup>
<MSBuildAllProjects Condition="'$(MSBuildVersion)' == '' Or '$(MSBuildVersion)' &lt; '16.0'">$(MSBuildAllProjects);$(MSBuildThisFileFullPath)</MSBuildAllProjects>
<HasSharedItems>true</HasSharedItems>
<SharedGUID>aa1dd2e8-1cde-4d1a-896c-8212c6d061a7</SharedGUID>
</PropertyGroup>
<PropertyGroup Label="Configuration">
<Import_RootNamespace>Scripts</Import_RootNamespace>
</PropertyGroup>
<ItemGroup>
<None Include="$(MSBuildThisFileDirectory)\scripts\*.fsx" />
</ItemGroup>
</Project>
Loading

0 comments on commit 65281d7

Please sign in to comment.