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.

Sign up for GitHub

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

Use Roslyn's editorconfig support #1771

Merged
merged 6 commits into from
Apr 21, 2020
Merged

Use Roslyn's editorconfig support #1771

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
32291fd
Use Roslyn's editorconfig support
JoeRobich Apr 18, 2020
18fada3
Update project when .editorconfig files change
JoeRobich Apr 21, 2020
140dd8d
Add tests for .editorconfig analyzer configuration
JoeRobich Apr 21, 2020
98abdc0
Merge branch 'master' into use-roslyn-editorconfig
JoeRobich Apr 21, 2020
9638ac8
Only add AnalyzerConfig if EditorConfig is enabled
JoeRobich Apr 21, 2020
a8ade9b
Update changelog
JoeRobich Apr 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions src/OmniSharp.Cake/CakeProjectSystem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,15 +284,17 @@ private ProjectInfo GetProject(CakeScript cakeScript, string filePath)
}

var projectId = ProjectId.CreateNewId(Guid.NewGuid().ToString());
var analyzerConfigDocuments = EditorConfigFinder
.GetEditorConfigPaths(filePath)
.Select(path =>
DocumentInfo.Create(
DocumentId.CreateNewId(projectId),
name: ".editorconfig",
loader: new FileTextLoader(path, Encoding.UTF8),
filePath: path))
.ToImmutableArray();
var analyzerConfigDocuments = _workspace.EditorConfigEnabled
? EditorConfigFinder
.GetEditorConfigPaths(filePath)
.Select(path =>
DocumentInfo.Create(
DocumentId.CreateNewId(projectId),
name: ".editorconfig",
loader: new FileTextLoader(path, Encoding.UTF8),
filePath: path))
.ToImmutableArray()
: ImmutableArray<DocumentInfo>.Empty;

return ProjectInfo.Create(
id: projectId,
Expand Down
2 changes: 2 additions & 0 deletions src/OmniSharp.Host/WorkspaceInitializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ public static void Initialize(IServiceProvider serviceProvider, CompositionHost
projectEventForwarder.Initialize();
var projectSystems = compositionHost.GetExports<IProjectSystem>();

workspace.EditorConfigEnabled = options.CurrentValue.FormattingOptions.EnableEditorConfigSupport;
Copy link
Member

Choose a reason for hiding this comment

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

it would be good to also reset this flag here https://github.com/OmniSharp/omnisharp-roslyn/blob/master/src/OmniSharp.Host/WorkspaceInitializer.cs#L62-L65
this way it should be effective in real time when options change (since they are watched)

Copy link
Member Author

Choose a reason for hiding this comment

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

This would be useful if there were a way to force all the projects to update. Is this option worth all this extra complexity? I would assume the presence of an .editorconfig would be the indicator of whether a user wanted .editorconfig support.

Copy link
Member

Choose a reason for hiding this comment

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

you are right we can solve this systematically separately as there are other areas that would benefit from that


foreach (var projectSystem in projectSystems)
{
try
Expand Down
14 changes: 11 additions & 3 deletions src/OmniSharp.MSBuild/ProjectManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,10 +377,13 @@ private void WatchProjectFiles(ProjectFileInfo projectFileInfo)
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: true, projectFileInfo.ProjectIdInfo);
});

_fileSystemWatcher.Watch(".editorconfig", (file, changeType) =>
if (_workspace.EditorConfigEnabled)
{
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo);
});
_fileSystemWatcher.Watch(".editorconfig", (file, changeType) =>
{
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo);
});
}

if (projectFileInfo.RuleSet?.FilePath != null)
{
Expand Down Expand Up @@ -498,6 +501,11 @@ private void UpdateAdditionalFiles(Project project, IList<string> additionalFile

private void UpdateAnalyzerConfigFiles(Project project, IList<string> analyzerConfigFiles)
Copy link
Member

Choose a reason for hiding this comment

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

should this code be conditioned on editorconfig being enabled in OmniSharp?
otherwise, if you disable it, it would be respected on formatting, but the analyzer options would still apply

{
if (!_workspace.EditorConfigEnabled)
{
return;
}

var currentAnalyzerConfigDocuments = project.AnalyzerConfigDocuments;
JoeRobich marked this conversation as resolved.
Show resolved Hide resolved
foreach (var document in currentAnalyzerConfigDocuments)
{
Expand Down
1 change: 1 addition & 0 deletions src/OmniSharp.Roslyn.CSharp/Workers/Formatting/FormattingWorker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ public static async Task<IEnumerable<LinePositionSpanTextChange>> GetFormattedTe

private static async Task<Document> FormatDocument(Document document, OmniSharpOptions omnisharpOptions, ILoggerFactory loggerFactory, TextSpan? textSpan = null)
{
// If we are not using .editorconfig for formatting options then we can avoid any overhead of calculating document options.
var optionSet = omnisharpOptions.FormattingOptions.EnableEditorConfigSupport
? await document.GetOptionsAsync()
JoeRobich marked this conversation as resolved.
Show resolved Hide resolved
: document.Project.Solution.Options;
Expand Down
6 changes: 6 additions & 0 deletions src/OmniSharp.Roslyn/OmniSharpWorkspace.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ namespace OmniSharp
public class OmniSharpWorkspace : Workspace
{
public bool Initialized { get; set; }
public bool EditorConfigEnabled { get; set; }
public BufferManager BufferManager { get; private set; }

private readonly ILogger<OmniSharpWorkspace> _logger;
Expand Down Expand Up @@ -105,6 +106,11 @@ public DocumentId TryAddMiscellaneousDocument(string filePath, string language)
var documentId = AddDocument(projectInfo.Id, filePath);
_logger.LogInformation($"Miscellaneous file: {filePath} added to workspace");

if (!EditorConfigEnabled)
{
return documentId;
}

var analyzerConfigFiles = projectInfo.AnalyzerConfigDocuments.Select(document => document.FilePath);
var newAnalyzerConfigFiles = EditorConfigFinder
.GetEditorConfigPaths(filePath)
Expand Down
4 changes: 2 additions & 2 deletions src/OmniSharp.Script/ScriptContextProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public ScriptContextProvider(ILoggerFactory loggerFactory, IOmniSharpEnvironment
});
}

public ScriptContext CreateScriptContext(ScriptOptions scriptOptions, string[] allCsxFiles)
public ScriptContext CreateScriptContext(ScriptOptions scriptOptions, string[] allCsxFiles, bool editorConfigEnabled)
{
var currentDomainAssemblies = AppDomain.CurrentDomain.GetAssemblies();

Expand Down Expand Up @@ -134,7 +134,7 @@ public ScriptContext CreateScriptContext(ScriptOptions scriptOptions, string[] a
AddMetadataReference(metadataReferences, inheritedCompileLib.Location);
}

var scriptProjectProvider = new ScriptProjectProvider(scriptOptions, _env, _loggerFactory, isDesktopClr);
var scriptProjectProvider = new ScriptProjectProvider(scriptOptions, _env, _loggerFactory, isDesktopClr, editorConfigEnabled);

return new ScriptContext(scriptProjectProvider, metadataReferences, compilationDependencies, _defaultGlobalsType);
}
Expand Down
24 changes: 14 additions & 10 deletions src/OmniSharp.Script/ScriptProjectProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ public class ScriptProjectProvider
private readonly IOmniSharpEnvironment _env;
private readonly ILogger _logger;
private readonly bool _isDesktopClr;
private readonly bool _editorConfigEnabled;

public ScriptProjectProvider(ScriptOptions scriptOptions, IOmniSharpEnvironment env, ILoggerFactory loggerFactory, bool isDesktopClr)
public ScriptProjectProvider(ScriptOptions scriptOptions, IOmniSharpEnvironment env, ILoggerFactory loggerFactory, bool isDesktopClr, bool editorConfigEnabled)
{
_scriptOptions = scriptOptions ?? throw new ArgumentNullException(nameof(scriptOptions));
_env = env ?? throw new ArgumentNullException(nameof(env));
Expand All @@ -61,6 +62,7 @@ public ScriptProjectProvider(ScriptOptions scriptOptions, IOmniSharpEnvironment
_compilationOptions = new Lazy<CSharpCompilationOptions>(CreateCompilationOptions);
_commandLineArgs = new Lazy<CSharpCommandLineArguments>(CreateCommandLineArguments);
_isDesktopClr = isDesktopClr;
_editorConfigEnabled = editorConfigEnabled;
}

private CSharpCommandLineArguments CreateCommandLineArguments()
Expand Down Expand Up @@ -162,15 +164,17 @@ public ProjectInfo CreateProject(string csxFileName, IEnumerable<MetadataReferen
}

var projectId = ProjectId.CreateNewId();
var analyzerConfigDocuments = EditorConfigFinder
.GetEditorConfigPaths(csxFilePath)
.Select(path =>
DocumentInfo.Create(
DocumentId.CreateNewId(projectId),
name: ".editorconfig",
loader: new FileTextLoader(path, Encoding.UTF8),
filePath: path))
.ToImmutableArray();
var analyzerConfigDocuments = _editorConfigEnabled
? EditorConfigFinder
.GetEditorConfigPaths(csxFilePath)
.Select(path =>
DocumentInfo.Create(
DocumentId.CreateNewId(projectId),
name: ".editorconfig",
loader: new FileTextLoader(path, Encoding.UTF8),
filePath: path))
.ToImmutableArray()
: ImmutableArray<DocumentInfo>.Empty;

var project = ProjectInfo.Create(
filePath: csxFilePath,
Expand Down
2 changes: 1 addition & 1 deletion src/OmniSharp.Script/ScriptProjectSystem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public void Initalize(IConfiguration configuration)
// Nothing to do if there are no CSX files
var allCsxFiles = _fileSystemHelper.GetFiles("**/*.csx").ToArray();

_scriptContext = new Lazy<ScriptContext>(() => _scriptContextProvider.CreateScriptContext(_scriptOptions, allCsxFiles));
_scriptContext = new Lazy<ScriptContext>(() => _scriptContextProvider.CreateScriptContext(_scriptOptions, allCsxFiles, _workspace.EditorConfigEnabled));

if (allCsxFiles.Length == 0)
{
Expand Down
2 changes: 1 addition & 1 deletion tests/TestUtility/TestHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public static OmniSharpWorkspace CreateCsxWorkspace(TestFile testFile)
public static void AddCsxProjectToWorkspace(OmniSharpWorkspace workspace, TestFile testFile)
{
var references = GetReferences();
var scriptHelper = new ScriptProjectProvider(new ScriptOptions(), new OmniSharpEnvironment(), new LoggerFactory(), true);
var scriptHelper = new ScriptProjectProvider(new ScriptOptions(), new OmniSharpEnvironment(), new LoggerFactory(), isDesktopClr: true, editorConfigEnabled: true);
var project = scriptHelper.CreateProject(testFile.FileName, references.Union(new[] { MetadataReference.CreateFromFile(typeof(CommandLineScriptGlobals).GetTypeInfo().Assembly.Location) }), testFile.FileName, typeof(CommandLineScriptGlobals), Enumerable.Empty<string>());
workspace.AddProject(project);

Expand Down