[
https://issues.apache.org/jira/browse/LUCENENET-341?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12835832#action_12835832
]
Andy Pook commented on LUCENENET-341:
-------------------------------------
Changing ISerializable ctors to private was confirmed by running
_SupportClass.TestBooleanQuerySerialization nunit test which is marked as
testing LUCENENET-338 mentioned by Digy in original email thread
> 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
> Attachments: LUCENENET-341-CharArraySet.patch,
> LUCENENET-341-NumericRangeQuery.patch,
> LUCENENET-341-SupportClass.ThreadClass.patch, LUCENENET-341-Term.patch
>
>
> 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" <[email protected]>
> 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 <[email protected]> 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.