Skip to content

Fix emitting hidden warnings #940

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

Merged
merged 2 commits into from
Feb 22, 2024
Merged

Fix emitting hidden warnings #940

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
31768f0
Close https://github.com/neo-project/neo-devpack-dotnet/issues/751
shargon Feb 22, 2024
19e5439
Optimize Success
shargon Feb 22, 2024
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
4 changes: 2 additions & 2 deletions src/Neo.Compiler.CSharp/CompilationContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public class CompilationContext
private readonly Dictionary<ITypeSymbol, byte> vtables = new(SymbolEqualityComparer.Default);
private byte[]? script;

public bool Success => diagnostics.All(p => p.Severity != DiagnosticSeverity.Error);
public bool Success => !diagnostics.Any(p => p.Severity == DiagnosticSeverity.Error);
Copy link
Contributor

@Jim8y Jim8y Feb 23, 2024

Choose a reason for hiding this comment

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

Hey, why this change? aren't them exactly the same?

Copy link
Member Author

@shargon shargon Feb 23, 2024

Choose a reason for hiding this comment

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

yes xD

Copy link
Member

@vncoelho vncoelho Feb 23, 2024

Choose a reason for hiding this comment

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

the change makes more readable I think

public IReadOnlyList<Diagnostic> Diagnostics => diagnostics;
public string? ContractName => displayName ?? Options.BaseName ?? className;
private string? Source { get; set; }
Expand Down Expand Up @@ -124,7 +124,7 @@ private void Compile()
foreach (SyntaxTree tree in compilation.SyntaxTrees)
{
SemanticModel model = compilation.GetSemanticModel(tree);
diagnostics.AddRange(model.GetDiagnostics());
diagnostics.AddRange(model.GetDiagnostics().Where(u => u.Severity != DiagnosticSeverity.Hidden));
Copy link
Member

@vncoelho vncoelho Feb 22, 2024

Choose a reason for hiding this comment

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

what this Hidden means?

Copy link
Member Author

@shargon shargon Feb 22, 2024

Choose a reason for hiding this comment

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

Something that we don't need to show to the user :)

Copy link
Member

@vncoelho vncoelho Feb 22, 2024

Choose a reason for hiding this comment

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

"These are typically used to provide suggestions or insights that could improve the code's quality, readability, or performance but are not critical errors or warnings that must be addressed for the code to run correctly."

In this case we are not talking about users, @shargon
Devs are using these tools

Copy link
Member

@vncoelho vncoelho Feb 22, 2024

Choose a reason for hiding this comment

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

In think the success change is good

Copy link
Member Author

@shargon shargon Feb 22, 2024

Choose a reason for hiding this comment

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

"These are typically used to provide suggestions or insights that could improve the code's quality, readability, or performance but are not critical errors or warnings that must be addressed for the code to run correctly."

In this case we are not talking about users, @shargon Devs are using these tools

Yes, devs hahah, but we don't need to show them these notifications #751

if (!Success) continue;
try
{
Expand Down