Neuer FixInsight ist da!

[Der schöne Günther]

How can one surpress FixInsight warnings locally? I might, for example, leave an overridden virtual method empty on purpose. FixInsight is correctly raising a W519 here but the code is entirely valid and working as intended.


Oh, and while we're at it: There must be an option to exclude files. If you got some third party sources, gnugettext.pas for example, you're also going to get warnings on files you definitely don't want to modify.

I'm sure this is already on uservoice but I haven't bothered to check


One last thing: C104 ("Class name should start with 'T'") is true, but I suppose it doesn't apply to attributes.
Embarcaderos examples omit the "T".
Also, Marco Cantus new book states

Zitat:

So if you name your class SimpleAttribute you'll be able to use in the code an attribute called Simple or SimpleAttribute . For this is the reason the classic initial T for Object Pascal classes is generally not used in case of attributes.

[Roman Yankovsky]

Zitat von Der schöne Günther:

How can one surpress FixInsight warnings locally?

You may use _FIXINSIGHT_ compiler conditional. It works just like a real compiler directive.

For example,

Delphi-Quellcode:

{$IFNDEF _FIXINSIGHT_}
  ThisCodeWillBeIgnored(1,2,3);
{$ENDIF}

I have to add this to the documentation. I have never expected that so many people will need this functionality

Zitat von Der schöne Günther:

There must be an option to exclude files. If you got some third party sources, gnugettext.pas for example, you're also going to get warnings on files you definitely don't want to modify.

Yes, this would be great. It's on my todo list.

Zitat von Der schöne Günther:

One last thing: C104 ("Class name should start with 'T'") is true, but I suppose it doesn't apply to attributes.

Agree. I will fix this

[Der schöne Günther]

Thank you for your reply.

Regarding excluding code from being checked, you mean like this?
Take this:

Delphi-Quellcode:

unit SomeUnit;

interface

type
   TParent = class
      procedure someVirtualMethod(); virtual;
   end;

   TChild = class(TParent)
      procedure someVirtualMethod(); override;
   end;

implementation

{ TParent }

procedure TParent.someVirtualMethod();
begin
   WriteLn('Quack!');
end;

{ TChild }

{$IFNDEF _FIXINSIGHT_}
procedure TChild.someVirtualMethod();
begin
   // No quack at all
end;
{$ENDIF _FIXINSIGHT_}

end.

Do you think it would also be possible to further specify which warning or convention to ignore? In this case, I'd only like to ignore a W519 but lose all the other cool warnings and conventions as well.

[Stevie]

Zitat von Der schöne Günther:

Do you think it would also be possible to further specify which warning or convention to ignore? In this case, I'd only like to ignore a W519 but lose all the other cool warnings and conventions as well.

Given how DelphiAST works I think that is not so trivial. The trick with the $IFNDEF just works because it ignores that part of code because the define is set by FixInsight.
For using the same technique it would require multiple parses for each rule or including ifdefs into the AST which currently is not the case.

A different approach could be like rule suppressions in StyleCop work.
But using attributes for such purpose will lead to W1025 when compiling that code unless you add a unit with dummy attributes.

Edit: Ok, maybe a combination of both could work... something like this:

Delphi-Quellcode:

{$IFDEF _FIXINSIGHT_}[SuppressRule(W519)]{$ENDIF}
procedure TChild.someVirtualMethod();
begin
   // No quack at all
end;

[Uwe Raabe]

Zitat von Stevie:

Delphi-Quellcode:

{$IFDEF _FIXINSIGHT_}[SuppressRule(W519)]{$ENDIF}
procedure TChild.someVirtualMethod();
begin
   // No quack at all
end;

How long is this "SuppressRule" valid: Only for the following method or for the rest of the unit?

We have a similar problem when dealing with string constants and localization tools automagically extracting those from the sources. It is quite common for these tools to accept a special formatted comment to ignore such a constant. If you scan the Delphi source you will find lots of comments like { Do not localize } which are valid for the current line. Perhaps we can have a similar approach for FixInsight warnings.

Delphi-Quellcode:

procedure TChild.someVirtualMethod(); { FIXINSIGHT: SuppressRule(W519) }
begin
   // No quack at all
end;

As the comment lies inside the method implementation the scope should be obvious. Depending on the nature of any rule other intrinsic scopes may be appropriate.

[Stevie]

Zitat von Uwe Raabe:

How long is this "SuppressRule" valid: Only for the following method or for the rest of the unit?

For exactly the element it was put on. In this case for the entire method. Personally I would not make it too fine grained.
In the end we want to find bad code and not put suppress rule directives all over the place, right?

Zitat von Uwe Raabe:

We have a similar problem when dealing with string constants and localization tools automagically extracting those from the sources. It is quite common for these tools to accept a special formatted comment to ignore such a constant. If you scan the Delphi source you will find lots of comments like { Do not localize } which are valid for the current line. Perhaps we can have a similar approach for FixInsight warnings.

...

As the comment lies inside the method implementation the scope should be obvious. Depending on the nature of any rule other intrinsic scopes may be appropriate.

As I said, the current implementation of DelphiAST limits the possibilities. Comments won't work either since they are not part of the generated AST (afaik).
To my knowledge FI works on the generated AST. Anything that is not part of that AST cannot be taken into consideration.

[Roman Yankovsky]

Zitat von Der schöne Günther:

Regarding excluding code from being checked, you mean like this?
Take this:

Yes, exactly like this.

Regarding your second question about ignoring only a specific warning Stefan is right, it's not a trivial task. I'm thinking about implementing this though.

Zitat von Stevie:

Zitat von Der schöne Günther:

Do you think it would also be possible to further specify which warning or convention to ignore? In this case, I'd only like to ignore a W519 but lose all the other cool warnings and conventions as well.

Given how DelphiAST works I think that is not so trivial. The trick with the $IFNDEF just works because it ignores that part of code because the define is set by FixInsight.
For using the same technique it would require multiple parses for each rule or including ifdefs into the AST which currently is not the case.

I was thinking about something like this:

Delphi-Quellcode:

{$FIOFF W509}
procedure Method;
begin

end;
{$FION W509}

It's still not trivial, but I think I can handle this directives in DelphiAST by adding a certan attribute to all syntax nodes between $FIOFF and $FION. Not in DelphiAST itself of course, but it is possible to inherit it and override a few methods. And as you have mentioned above, attributes is another option.

[Der schöne Günther]

I don't know about this DelphiAST, but what about the other way around? Just parse as you always do. We now have a list of warnings and the line number it occurred on.

So in case the code goes like this

Delphi-Quellcode:

25:   procedure TChild.someVirtualMethod();
26:   begin
27:       // No quack at all
28:   end;

, there is a W519 raised for line 25. Now before outputting our results to the IDE's listbox, let's check line 25 whether it contains a comment with an exclamation mark followed by "W519".

So if the code is like

Delphi-Quellcode:

25:   procedure TChild.someVirtualMethod(); //!W519 because FixInsight is smart
26:   begin
27:       // No quack at all
28:   end;

, there will be no "false positive".

[Roman Yankovsky]

Nice idea

[Roman Yankovsky]

Finally FixInsight 2015.10 is able to suppress a warning for a piece of code (that was discussed on a previous page). It works not for a scope, but for a particular line though.

Example:

Delphi-Quellcode:

      procedure RestartTimer;
      begin
        FTimer.Enabled := False;
        FTimer.Enabled := True; //FI:W508 - this warning will be ignored.
      end;

So it's just a "FI:<RULENUMBER>" comment.