On Sat, 2008-12-27 at 16:43 +0800, Cedric Vivier wrote:
> On Sat, Dec 27, 2008 at 3:46 AM, Sebastien Pouliot
> <[email protected]> wrote:
> > + foreach (var local in suspectLocals)
> {
> > + string msg = string.Format
> ("Local {0}is not disposed (at least not locally).",
> GetFriendlyNameOrEmpty (local.GetVariable (method)));
>
> Is it really helpful not to display V_* ? maybe adding the
> type of the
> variable would be useful (always or when the name is not
> available).
> E.g. "Local of type 'Disposable' is no disposed ..."
>
> Nice idea.
> I think not displaying V_* is helpful since the goal of variable name
> in the text is to help localize it -or- even remember which and what
> it does. As such displaying V_* would be just noise since it doesn't
> really help (but now we still get some help thanks to the type).
V_* is not helpful itself, except that it can give an hint if the defect
is about the same variable used more than once (i.e. same name). However
with the type we already give a more useful hint imo.
> By the way, this "feature" could be useful in the runner itself i
> think, currently we use the instruction reported to display only the
> line number (if available) but for rules like this about the usage of
> a local, it would be nice imo to show additional information like: the
> defect is about a local's usage, its name, and its type, by default
> for all rules reporting on an instruction where GetVariable rock
> returns non-null.
Yes, or we could add overloads (well change the current one accepting an
Instruction) to accept any IMetadataTokenProvider - it now only allows
MethodDefinition.
So this could become
Runner.Report (variable, instruction, severity, confidence);
which under the hood would construct a defect with
Defect (rule, target*, variable, instruction, severity, confidence);
* where target is the method for an IMethodRule (so this information is
not lost).
Now this would not work for an ITypeRule where you analyze a method -
but this *should* work (minus the extra check) using existing code.
> It would also allow generalized highlighting/filtering of a particular
> local type or name (by grep or whatever) - like "hmm ok let's
> fix/review FileStream-related problems first".
>
> Updated patch according to review.
Please commit!
Sebastien
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Gendarme" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/gendarme?hl=en
-~----------~----~----~----~------~----~------~--~---
