Fix "Compiler warnings to worry about"
--------------------------------------

                 Key: LUCENENET-341
                 URL: https://issues.apache.org/jira/browse/LUCENENET-341
             Project: Lucene.Net
          Issue Type: Bug
            Reporter: Andy Pook
            Priority: Minor


>From email thread...

Subscribed

1- Ah, hadn't spotted ISerializable. However, the recommendation for
combining ISerializable and sealed classes is to use private instead
of protected. (http://msdn.microsoft.com/en-us/library/ms182343(VS.80).aspx
- "For a sealed class, make the constructor private; otherwise, make
it protected.")

Should I create a separate issue for each or a single issue with three patches?

--Andy

>From    "Digy" <digyd...@gmail.com>
Subject RE: Compiler warnings to worry about?
Date    Thu, 18 Feb 2010 17:06:39 GMT
Hi Andy,

1- No we can not remove the protected constructors. They are used in
serialization process via reflection. See the issue
http://issues.apache.org/jira/browse/LUCENENET-338 .

2- You are correct in general, but no danger in this case
(SupportClass.ThreadClass)

3- Yes, we should add the override/new keyword.

Your patch is welcome.

PS: Please subscribe to mailing list to send mails. Otherwise I'll have to
do a allow/deny confirmation (as a moderator)  with every mail you send and
it can get lost among spams.

DIGY
- Hide quoted text -




On 18 February 2010 10:35, Andy Pook <andy.p...@gmail.com> wrote:
>
> We get a lot of compiler warnings. This has been discussed before and the 
> result seemed to be that it would be a lot of effort and would make it 
> difficult to maintain across versions as we have previously always used the 
> automated process. And as most of them are just to do with xml comments it's 
> never been much to worry about.
> However, we do have some warnings that I think we should worry about.
> I normally compile using msbuild from the command line. This makes it easier 
> for me to ignore most of the warnings and just see the interesting stuff.
> msbuild 
> -p:NoWarn="0168,0169,0414,0612,0618,0649,1572,1573,1574,1580,1587,1591"
> Other options are added for the Release build.
> Here are the warnings. (Forgive the line breaks, I just cut/pasted from cmd).
> "C:\projects\lucene.net\trunk\src\Lucene.Net\Lucene.Net.sln" (default target) 
> (1) ->
> "C:\projects\lucene.net\trunk\src\Lucene.Net\Lucene.Net.csproj" (default 
> target) (2) ->
> (CoreCompile target) ->
>   Index\Term.cs(174,19): warning CS0628: 
> 'Lucene.Net.Index.Term.Term(System.Run
> time.Serialization.SerializationInfo, 
> System.Runtime.Serialization.StreamingCon
> text)': new protected member declared in sealed class
>   Search\NumericRangeQuery.cs(389,19): warning CS0628: 
> 'Lucene.Net.Search.Numer
> icRangeQuery.NumericRangeQuery(System.Runtime.Serialization.SerializationInfo,
> System.Runtime.Serialization.StreamingContext)': new protected member declared
> in sealed class
>   SupportClass.cs(132,18): warning CS0659: 'SupportClass.ThreadClass' 
> overrides
>  Object.Equals(object o) but does not override Object.GetHashCode()
>   SupportClass.cs(132,18): warning CS0661: 'SupportClass.ThreadClass' defines 
> o
> perator == or operator != but does not override Object.GetHashCode()
>   Analysis\CharArraySet.cs(449,29): warning CS0114: 
> 'Lucene.Net.Analysis.CharAr
> raySet.Clear()' hides inherited member 'System.Collections.Hashtable.Clear()'.
> To make the current member override that implementation, add the override 
> keywo
> rd. Otherwise add the new keyword.
>   Analysis\CharArraySet.cs(418,16): warning CS0114: 
> 'Lucene.Net.Analysis.CharAr
> raySet.UnmodifiableCharArraySet.AddAll(System.Collections.ICollection)' hides 
> i
> nherited member 
> 'Lucene.Net.Analysis.CharArraySet.AddAll(System.Collections.ICo
> llection)'. To make the current member override that implementation, add the 
> ov
> erride keyword. Otherwise add the new keyword.
>   QueryParser\QueryParser.cs(1421,4): warning CS0162: Unreachable code 
> detected
>   QueryParser\QueryParser.cs(1469,4): warning CS0162: Unreachable code 
> detected
>   QueryParser\QueryParser.cs(1482,4): warning CS0162: Unreachable code 
> detected
>   QueryParser\QueryParser.cs(1542,4): warning CS0162: Unreachable code 
> detected
>   QueryParser\QueryParser.cs(1633,4): warning CS0162: Unreachable code 
> detected
>   QueryParser\QueryParser.cs(1984,4): warning CS0162: Unreachable code 
> detected
>   Search\Filter.cs(42,7): warning CS1570: XML comment on 
> 'Lucene.Net.Search.Fil
> ter.Bits(Lucene.Net.Index.IndexReader)' has badly formed XML -- 'A name 
> contain
> ed an invalid character.'
>   Util\Version.cs(67,41): warning CS1570: XML comment on 
> 'Lucene.Net.Util.Versi
> on.LUCENE_29' has badly formed XML -- 'Whitespace is not allowed at this 
> locati
> on.'
>     14 Warning(s)
> The QueryParser ones we probably don't want to get into as it's generated 
> code (maybe for a later date).
> The CS1570 comment warnings
> That leaves 3 basic types of problem:
>
> CS0628 "New protected member declared in a sealed class"
> If a class is sealed, you cannot inherit from it, therefore there can be no 
> descendants to use it. The usual solution is make them private.
> Both of these warnings are constructors and don't seem to be referenced from 
> anywhere.
> Suggestion: Just remove them.
> CS0659 "Overrides Equals but not GetHashCode"
> Not doing this can lead to some unexpected consequences. especially is the 
> object is added to a collection that depends on hash codes.
> A GetHashCode can usually be fairly easily derived from the Equals.
> CS0114 "hides inherited member"
> This can lead to some weird consequences esp. if polymorphism is used.
> Added 'override' is almost always the right solution.
>
> I would consider these as fairly serious smells.I can't see any reason why we 
> should not fix these at this stage. I can provide patches if the committers 
> are interested.
> --Andy Pook

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to